lkml.org 
[lkml]   [2017]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 19/19] fs: handle inode->i_version more efficiently
    On Mon 18-12-17 10:11:56, Jeff Layton wrote:
    > static inline bool
    > inode_maybe_inc_iversion(struct inode *inode, bool force)
    > {
    > - atomic64_t *ivp = (atomic64_t *)&inode->i_version;
    > + u64 cur, old, new;
    >
    > - atomic64_inc(ivp);
    > + cur = (u64)atomic64_read(&inode->i_version);
    > + for (;;) {
    > + /* If flag is clear then we needn't do anything */
    > + if (!force && !(cur & I_VERSION_QUERIED))
    > + return false;

    The fast path here misses any memory barrier. Thus it seems this query
    could be in theory reordered before any store that happened to modify the
    inode? Or maybe we could race and miss the fact that in fact this i_version
    has already been queried? But maybe there's some higher level locking that
    makes sure this is all a non-issue... But in that case it would deserve
    some comment I guess.

    > +
    > + /* Since lowest bit is flag, add 2 to avoid it */
    > + new = (cur & ~I_VERSION_QUERIED) + I_VERSION_INCREMENT;
    > +
    > + old = atomic64_cmpxchg(&inode->i_version, cur, new);
    > + if (likely(old == cur))
    > + break;
    > + cur = old;
    > + }
    > return true;
    > }
    >

    ...

    > static inline u64
    > inode_query_iversion(struct inode *inode)
    > {
    > - return inode_peek_iversion(inode);
    > + u64 cur, old, new;
    > +
    > + cur = atomic64_read(&inode->i_version);
    > + for (;;) {
    > + /* If flag is already set, then no need to swap */
    > + if (cur & I_VERSION_QUERIED)
    > + break;
    > +
    > + new = cur | I_VERSION_QUERIED;
    > + old = atomic64_cmpxchg(&inode->i_version, cur, new);
    > + if (old == cur)
    > + break;
    > + cur = old;
    > + }

    Why not just use atomic64_or() here?

    Honza
    --
    Jan Kara <jack@suse.com>
    SUSE Labs, CR

    \
     
     \ /
      Last update: 2017-12-18 17:35    [W:3.319 / U:0.228 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site