linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Tejun Heo <tj@kernel.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>,
	Kay Sievers <kay@vrfy.org>,
	linux-next@vger.kernel.org
Subject: Re: [BUG -next] sysfs change breaks userspace
Date: Fri, 1 Nov 2013 09:14:43 -0700	[thread overview]
Message-ID: <20131101161443.GA4593@kroah.com> (raw)
In-Reply-To: <20131101150800.GE20005@htj.dyndns.org>

On Fri, Nov 01, 2013 at 11:08:00AM -0400, Tejun Heo wrote:
> Hello, Greg.
> 
> On Fri, Nov 01, 2013 at 08:04:48AM -0700, Greg Kroah-Hartman wrote:
> > > @@ -800,7 +800,7 @@ EXPORT_SYMBOL_GPL(sysfs_notify);
> > >  const struct file_operations sysfs_file_operations = {
> > >  	.read		= seq_read,
> > >  	.write		= sysfs_write_file,
> > > -	.llseek		= seq_lseek,
> > > +	.llseek		= generic_file_llseek,
> > 
> > Does that mean that seq_lseek can't handle SEEK_END?  Shouldn't we fix
> > that instead?
> 
> As the files are dynamic, seq_file has no way of knowing whether the
> file is gonna end at all.  It's perfectly valid to have an infinite
> file.  The only benefits of using seq_lseek() instead of
> generic_file_llseek() are
> 
> * Early failure.  For whatever reason, traversing to certain file
>   position should fail, seq_lseek() will report such failure on
>   seek(2) instead of the following read/write operations.
> 
> * EOF detection.  While SEEK_END is not supported, SEEK_SET/CUR +
>   large offset can be used to detect eof - eof at the time of the last
>   seek anyway as the file size may change dynamically.
> 
> I don't think either would matter for sysfs or for prospect kernfs
> users and sticking with genefic_file_llseek() should be fine.

Ok, you are right, that makes sense as sysfs files do have an end,
although it seems odd that userspace would care to try to find it and
not just do a seek to the start of the file.

Fun userspace oddities, just when you thought you had seen it all...

thanks,

greg k-h

  reply	other threads:[~2013-11-01 16:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-31 11:43 [BUG -next] sysfs change breaks userspace Heiko Carstens
2013-10-31 17:25 ` Tejun Heo
2013-11-01 14:13   ` Heiko Carstens
2013-11-01 14:35     ` Tejun Heo
2013-11-01 15:04       ` Greg Kroah-Hartman
2013-11-01 15:08         ` Tejun Heo
2013-11-01 16:14           ` Greg Kroah-Hartman [this message]
2013-11-01 17:16             ` [PATCH driver-core-next] sysfs: use generic_file_llseek() for sysfs_file_operations Tejun Heo
2013-11-01 17:19               ` Greg Kroah-Hartman
2013-11-01 18:40                 ` Heiko Carstens

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=20131101161443.GA4593@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=kay@vrfy.org \
    --cc=linux-next@vger.kernel.org \
    --cc=tj@kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).