All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: Dave Chinner <david@fromorbit.com>
Cc: Ben Myers <bpm@sgi.com>, xfs@oss.sgi.com
Subject: Re: [PATCH 0/5] xfs: more patches for 3.13
Date: Mon, 18 Nov 2013 15:52:33 -0600	[thread overview]
Message-ID: <528A8C21.2070801@sandeen.net> (raw)
In-Reply-To: <20131117194830.GT6188@dastard>

On 11/17/13, 1:48 PM, Dave Chinner wrote:
> On Fri, Nov 15, 2013 at 11:55:20AM -0600, Eric Sandeen wrote:
>> On 11/15/13, 11:19 AM, Eric Sandeen wrote:
>>> On 11/13/13, 7:16 PM, Dave Chinner wrote:
>>>
>>>> That's client side, not server side, so that's the NFS client inode
>>>> it is locking, not the server side XFS inode.
>>>
>>> Ah, geez, you're right. (x3)
>>>
>>> <snip>
>>>
>>>> Server side, where i_version is pulled out of an XFS inode:
>>>>
>>>> $ git grep i_version fs/nfsd
>>>> fs/nfsd/nfs3xdr.c:      fhp->fh_post_change = fhp->fh_dentry->d_inode->i_version;
>>>> fs/nfsd/nfs4xdr.c:              write64(p, inode->i_version);
>>>> fs/nfsd/nfsfh.h:                fhp->fh_pre_change = inode->i_version;
>>>> $
>>>>
>>>> the nfsfh.h hit is in fill_pre_wcc(), which appears to be called
>>>> under i_mutex but not i_lock. The xdr encoding functions don't
>>>> appear to be holding i_lock, and may be holding i_mutex, but I
>>>> haven't looked that far.
>>>
>>> I'm still not sure how  . . . 
>>
>> ugh didn't mean to send this reply quite yet, sorry.
>>
>> Not sure how we do an unlocked read on a 32-bit machine that doesn't potentially
>> get the wrong answer.  I talked to Bruce about it a bit; nothing jumped out at
>> us.  At worst (?) it seems that if you happened to race on a read at exactly
>> the 2^32'nd modification, you might go backwards.  
>>
>> As Bruce says, even if so, maybe "so rare we don't care?"
> 
> Especially as it requires 2^32 modifications to first be made to the
> file before there's even the possibility of a high word race on a
> read and then there's only one increment we could race with before
> it doesn't chnge again for another 2^32 modifications.
> 
> Hence, at 1 in 4 billion modifications potentially causing a problem,
> I'd agree with the "so rare we don't care" assessment.

Ok, I'm sold.

-Eric

> Cheers,
> 
> Dave.
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2013-11-18 21:52 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-01  4:27 [PATCH 0/5] xfs: more patches for 3.13 Dave Chinner
2013-11-01  4:27 ` [PATCH 1/5] xfs: xfs_remove deadlocks due to inverted AGF vs AGI lock ordering Dave Chinner
2013-11-01  4:27 ` [PATCH 2/5] xfs: open code inc_inode_iversion when logging an inode Dave Chinner
2013-11-05 16:41   ` Christoph Hellwig
2013-11-18 21:54   ` Eric Sandeen
2013-11-18 22:28     ` Ben Myers
2013-11-18 22:45       ` Eric Sandeen
2013-11-01  4:27 ` [PATCH 3/5] xfs: trace AIL manipulations Dave Chinner
2013-11-05 16:41   ` Christoph Hellwig
2013-11-01  4:27 ` [PATCH 4/5] xfs: add tracepoints to AGF/AGI read operations Dave Chinner
2013-11-05 16:42   ` Christoph Hellwig
2013-11-01  4:27 ` [PATCH 5/5] xfs: increase inode cluster size for v5 filesystems Dave Chinner
2013-11-05 16:43   ` Christoph Hellwig
2013-11-05 19:56     ` Dave Chinner
2013-11-06 21:31       ` Ben Myers
2013-11-07  0:32         ` Dave Chinner
2013-11-12 17:33       ` Christoph Hellwig
2013-11-08 18:21   ` Eric Sandeen
2013-11-11 22:45     ` Dave Chinner
2013-11-12  0:24       ` Eric Sandeen
2013-11-14 18:51   ` Eric Sandeen
2013-11-06 23:01 ` [PATCH 0/5] xfs: more patches for 3.13 Ben Myers
2013-11-07  1:57   ` Dave Chinner
2013-11-13  1:16     ` Eric Sandeen
2013-11-14  1:16       ` Dave Chinner
2013-11-15 17:19         ` Eric Sandeen
2013-11-15 17:55           ` Eric Sandeen
2013-11-17 19:48             ` Dave Chinner
2013-11-18 21:52               ` Eric Sandeen [this message]
2013-11-18 20:30 ` Ben Myers

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=528A8C21.2070801@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=bpm@sgi.com \
    --cc=david@fromorbit.com \
    --cc=xfs@oss.sgi.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.