On Mon, Feb 1, 2010 at 9:22 PM, John Kacur <jkacur@xxxxxxxxxx> wrote:
> On Sun, Jan 31, 2010 at 6:29 AM, Dmitry Torokhov
> <dmitry.torokhov@xxxxxxxxx> wrote:
>> On Sun, Jan 31, 2010 at 05:20:55AM +0100, Arnd Bergmann wrote:
>>> On Sunday 31 January 2010, John Kacur wrote:
>>> > > Sorry, I should have been clearer, but not implementing llseek
>>> > > is the problem I was referring to: When a driver has no explicit
>>> > > .llseek operation in its file operations and does not call
>>> > > nonseekable_open from its open operation, the VFS layer will
>>> > > implicitly use default_llseek, which takes the BKL. We're
>>> > > in the process of changing drivers not to do this, one by one
>>> > > so we can kill the BKL in the end.
>>> > >
>>> >
>>> > I know we've discussed this before, but why wouldn't the following
>>> > make more sense?
>>> > .llseek = no_llseek,
>>>
>>> That's one of the possible solutions. Assigning it to generic_file_llseek
>>> also gets rid of the BKL but keeps the current behaviour (calling seek
>>> returns success without having an effect, no_llseek returns -ESPIPE),
>>> while calling nonseekable_open has the other side-effect of making
>>> pread/pwrite fail with -ESPIPE, which is more consistent than
>>> only failing seek.
>>>
>>
>> OK, so how about the patch below (on top of Thadeu's patch)?
>>
>> --
>> Dmitry
>>
>> Input: uinput - use nonseekable_open
>>
>> Seeking does not make sense for uinput so let's use nonseekable_open
>> to mark the device non-seekable.
>>
>> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxx>
>> ---
>>
>> drivers/input/misc/uinput.c | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>>
>> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
>> index 18206e1..7089151 100644
>> --- a/drivers/input/misc/uinput.c
>> +++ b/drivers/input/misc/uinput.c
>> @@ -278,6 +278,7 @@ static int uinput_create_device(struct uinput_device *udev)
>> static int uinput_open(struct inode *inode, struct file *file)
>> {
>> struct uinput_device *newdev;
>> + int error;
>>
>> newdev = kzalloc(sizeof(struct uinput_device), GFP_KERNEL);
>> if (!newdev)
>> @@ -291,6 +292,12 @@ static int uinput_open(struct inode *inode, struct file *file)
>>
>> file->private_data = newdev;
>>
>> + error = nonseekable_open(inode, file);
>> + if (error) {
>> + kfree(newdev);
>> + return error;
>> + }
>> +
>> return 0;
>> }
>>
>>
>
> Hmnn, if you look at nonseekable_open() it will always return 0. I
> think you can just do the following.
>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 18206e1..697c0a6 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -291,7 +291,7 @@ static int uinput_open(struct inode *inode, struct file *fil
>
> file->private_data = newdev;
>
> - return 0;
> + return nonseekable_open(inode, file);
> }
>
> Signed-off-by: John Kacur <jkacur@xxxxxxxxxx>
>
Btw, Thadeu Lima de Souza Cascardo should just combine that all into
one patch, no point really in making two patches out of that.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
