All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joel Becker <jlbec@evilplan.org>
To: ocfs2-devel@oss.oracle.com
Subject: [Ocfs2-devel] [PATCH] ocfs2: llseek requires to ocfs2 inode lock for the file in SEEK_END
Date: Sat, 29 Jun 2013 14:28:36 +0100	[thread overview]
Message-ID: <20130629132836.GK13405@ZenIV.linux.org.uk> (raw)
In-Reply-To: <51CD533D.9030503@oracle.com>

On Fri, Jun 28, 2013 at 05:11:25PM +0800, Jeff Liu wrote:
> On 06/28/2013 06:59 AM, Sunil Mushran wrote:
> 
> > The qs is whether this change is required for a real problem or not. If
> > so, what is that logic
> > that gets tripped up by this behaviour.

This is a straight correctness fix.  We take the hit and apply it.  I'll
check the actual patch on that email.

Joel

> 
> 
> IMHO, there have a couple of commands in Coreutils would be affected by this
> behavior:
> - tail(1) with "-c bytes" option.
> - wc(1): when counting only bytes, it will try to figure out the file size
> 	 through lseek (SEEK_END - SEEK_CUR) when counting only bytes.
> 
> Other popular programs need get the file size via SEEK_END only in some corner
> cases.  e.g, if the input file is not a regular file or the utility failed to
> fetch the file size via fstat(2), they will call lseek as an alternative approach,
> like split(1), truncate(1), head(1) with '-c bytes' option.
> 
> Generally, the user applications might fetch the file size through fstat(2) which
> will go through ocfs2_getattr() so that it's safe to us because we perform inode
> re-validation before filling up the generic attributes.
> 
> IOWs, the user application could be changed accordingly to avoid this issue.
> However, this most likely a workaround to some extent, and we have a bug for
> the SEEK_END business.
> 
> As per our current discussion, one big concern of us is regarding the performance
> with this fix, I'd like to consider it from another point of view, that is the user
> should take all the blame to themselves if they would like to run SEEK_END bounds
> operation on OCFS2, does it sounds make sense? :-P.
> If yes, it would be better to consolidate the code comments to indicate the
> performance negatively and it is not advisable to fetch file size via SEEK_END
> on OCFS2.
> 
> Thanks,
> -Jeff
> 
> > 
> > 
> > On Thu, Jun 27, 2013 at 3:08 PM, Andrew Morton
> > <akpm at linux-foundation.org <mailto:akpm@linux-foundation.org>> wrote:
> > 
> >     On Wed, 26 Jun 2013 20:34:19 -0700 Sunil Mushran
> >     <sunil.mushran at gmail.com <mailto:sunil.mushran@gmail.com>> wrote:
> > 
> >     > AFAIR, this behavior has been there since day 1 and changing it
> >     will impact
> >     > performance negatively. I would recommend against making this
> >     change for
> >     > one app.
> > 
> >     Well, it's a bug fix isn't it?  SEEK_END on a 15k file is presently
> >     returning
> >     10k.
> > 
> >     Obviously tradeoffs are involved - is there any way in which this
> >     behaviour can be fixed without serious performance damage?
> > 
> > 
> > 
> > 
> > _______________________________________________
> > Ocfs2-devel mailing list
> > Ocfs2-devel at oss.oracle.com
> > https://oss.oracle.com/mailman/listinfo/ocfs2-devel
> 
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel

-- 

  reply	other threads:[~2013-06-29 13:28 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-20  8:23 [Ocfs2-devel] [PATCH] ocfs2: llseek requires to ocfs2 inode lock for the file in SEEK_END shencanquan
2013-06-26 21:18 ` Andrew Morton
2013-06-27  1:19   ` shencanquan
2013-06-27  1:25     ` Andrew Morton
2013-06-27  1:50       ` shencanquan
2013-06-27  3:34         ` Sunil Mushran
2013-06-27  8:56           ` Jensen
2013-06-27 22:08           ` Andrew Morton
2013-06-27 22:59             ` Sunil Mushran
2013-06-28  9:11               ` Jeff Liu
2013-06-29 13:28                 ` Joel Becker [this message]
2013-06-29 13:37 ` Joel Becker
2013-07-01  1:38   ` Jensen
2013-07-02 19:58     ` Mark Fasheh
2013-07-02 21:19       ` Joel Becker

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=20130629132836.GK13405@ZenIV.linux.org.uk \
    --to=jlbec@evilplan.org \
    --cc=ocfs2-devel@oss.oracle.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.