lkml.org 
[lkml]   [2009]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: Migration of kernel interfaces to seq_files breaks pread() consumers
From
On Thu, Jan 29, 2009 at 7:01 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Paul Turner <pjt@google.com> writes:
>>
>> Thanks Eric,
>>
>> Moving the position into the seq_file structure is much cleaner. Basic
>> tests seem to work ok.
>
> Thanks.
>
>> Few comments:
>> - seq_open needs its fmode opened up to take advantage of this [patch
>> below]
>
> Good point.
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
>
>> - There's still the inherent problem of reads peturbing other reads (both
>> read and pread). Our applications should be ok with this, however it does
>> deviate from the supposedly thread-safe pread semantics.
>
> I don't see this not being thread safe.
>
> If the data that is returned is stable. We should not see anything
> changing. Otherwise it is all bets are off in any case.
>

With just reads you had stability on the record level at least.
Concurrent reads/seeks on a file made it obvious where things weren't
thread safe.

Obviously userspace doing stupid things isn't a kernel problem but it
could become one in the future if a previously safe concurrent pread
interface is migrated to use seqfile.

> Because seq_read happens under m->lock I don't see how there will
> be anything thread unsafe.
>
> The position that changes is just an internal implementation detail to
> keep track of which data that we have cached.

I'm only referring to unstable records since every time we seek we
trash the cache.

>
>>
>> - Paul
>>
>> ---
>> fs/seq_file.c | 2 --
>> 1 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/seq_file.c b/fs/seq_file.c
>> index cd63d69..aa3621f 100644
>> --- a/fs/seq_file.c
>> +++ b/fs/seq_file.c
>> @@ -48,8 +48,6 @@ int seq_open(struct file *file, const struct seq_operations
>> *op)
>> */
>> file->f_version = 0;
>>
>> - /* SEQ files support lseek, but not pread/pwrite */
>> - file->f_mode &= ~(FMODE_PREAD | FMODE_PWRITE);
>> return 0;
>> }
>> EXPORT_SYMBOL(seq_open);
>> --
>> 1.5.4.5
>


\
 
 \ /
  Last update: 2009-01-30 07:13    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site