From: ebiederm@xmission.com (Eric W. Biederman)
To: Paul Turner <pjt@google.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org
Subject: Re: Migration of kernel interfaces to seq_files breaks pread() consumers
Date: Thu, 29 Jan 2009 19:01:31 -0800 [thread overview]
Message-ID: <m1bptp8pkk.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0901291719520.26444@kitami.corp.google.com> (Paul Turner's message of "Thu\, 29 Jan 2009 17\:26\:52 -0800 \(PST\)")
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.
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
next prev parent reply other threads:[~2009-01-30 3:01 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-17 7:51 Migration of kernel interfaces to seq_files breaks pread() consumers Paul Turner
2009-01-25 2:19 ` Andrew Morton
2009-01-25 3:40 ` Paul Turner
2009-01-25 12:08 ` Alexey Dobriyan
2009-01-27 21:47 ` Eric W. Biederman
2009-01-27 21:48 ` [PATCH 1/2] seq_file: Move traverse so it can be used from seq_read Eric W. Biederman
2009-01-27 21:49 ` [PATCH 2/2] seq_file: Properly cope with pread Eric W. Biederman
2009-01-30 1:26 ` Migration of kernel interfaces to seq_files breaks pread() consumers Paul Turner
2009-01-30 3:01 ` Eric W. Biederman [this message]
2009-01-30 6:09 ` Paul Turner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m1bptp8pkk.fsf@fess.ebiederm.org \
--to=ebiederm@xmission.com \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pjt@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox