Re: Migration of kernel interfaces to seq_files breaks pread() consumers

From: Eric W. Biederman
Date: Thu Jan 29 2009 - 22:01:38 EST


Paul Turner <pjt@xxxxxxxxxx> 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@xxxxxxxxxxxx>


> - 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.

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.

>
> - 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/