lkml.org 
[lkml]   [2014]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] misc: always assign miscdevice to file->private_data in open()
Am 2014-10-08 15:43, schrieb Greg KH:
> On Wed, Oct 08, 2014 at 10:47:54AM +0200, Martin Kepplinger wrote:
>> As of now, a miscdevice driver has to provide an implementation of
>> the open() file operation if it wants to have misc_open() assign a
>> pointer to struct miscdevice to file->private_data for other file
>> operations to use (given the user calls open()).
>>
>> This leads to situations where a miscdevice driver that doesn't need
>> internal operations during open() has to implement open() that only
>> returns immediately, in order to use the data in private_data in other
>> fops.
>
> Yeah, that's messy, do we have any in-kernel misc drivers that do this?
>
>> This change provides consistent behaviour for miscdevice developers by
>> always providing the pointer in private_data. A driver's open() fop would,
>> of course, just overwrite it, when using private_data itself.
>>
>> Signed-off-by: Martin Kepplinger <martink@posteo.de>
>> ---
>> This is really only a question: Do I understand this correctly, and,
>> could this change then hurt any existing driver?
>
> I don't know, take a look at the existing ones and see please.
>
>> As a driver developer it took me a while to figure out what happens here,
>> and in my situation it would have been nice to just have this feature as
>> part of the miscdevice API. Possibly documented somewhere?
>
> Patches always accepted for documentation :)

What would be a good place for this?
Documentation/driver-model/device.txt or
Documentation/filesystem/vfs.txt like so? I'm not sure.

From facd10cfa7539755e960dec8cc009934200e68ec Mon Sep 17 00:00:00 2001
From: Martin Kepplinger <martink@posteo.de>
Date: Thu, 9 Oct 2014 14:54:28 +0200
Subject: [PATCH] documentation: misc_open sets private_data for driver's
open()

Signed-off-by: Martin Kepplinger <martink@posteo.de>
---
Documentation/filesystems/vfs.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/vfs.txt
b/Documentation/filesystems/vfs.txt
index 61d65cc..06df9d9 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -869,7 +869,8 @@ otherwise noted.
done the way it is because it makes filesystems simpler to
implement. The open() method is a good place to initialize the
"private_data" member in the file structure if you want to point
- to a device structure
+ to a device structure. In the case of "struct miscdevice", when
+ you implement open() this is done automatically.

flush: called by the close(2) system call to flush a file

--
1.7.10.4

>
>> misc_open() is called in any case, on open(). As long as miscdevice drivers
>> don't explicitly rely on private_data being NULL exactly IF they don't
>> implement an open() fop (which I wouldn't imagine), this would make things
>> even more convenient.
>
> I agree, but it would be great if you can audit the existing misc
> drivers to ensure we don't break anything with this change. Can you do
> that please?
>

I would grep -r "struct miscdevice" ./drivers/; and look at struct
file_operations of these results, see how their open() looks like, and
where they assign something to private_data.

If you have an idea for a script that lists all relevant files for me,
please tell me.

I queue this up but can't tell at all when it actually gets scheduled in ;)

I guess some do this work on their own because they don't know that
misc_open() already does it for them. It would probably be too much to
check what drivers could then just drop their open(). Interesting though
;) But in the short term, I think the appended documentation would help.

martin



\
 
 \ /
  Last update: 2014-10-09 15:43    [W:0.095 / U:0.692 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site