linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Brian Foster <bfoster@redhat.com>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>,
	Alex Lyakas <alex@zadarastorage.com>,
	linux-xfs@vger.kernel.org, libor.klepac@bcom.cz
Subject: Re: [PATCH] xfs: hold xfs_buf locked between shortform->leaf conversion and the addition of an attribute
Date: Fri, 11 Aug 2017 12:22:04 +1000	[thread overview]
Message-ID: <20170811022204.GQ21024@dastard> (raw)
In-Reply-To: <20170810183233.GG3777@bfoster.bfoster>

On Thu, Aug 10, 2017 at 02:32:33PM -0400, Brian Foster wrote:
> On Thu, Aug 10, 2017 at 10:55:48AM -0700, Darrick J. Wong wrote:
> > On Thu, Aug 10, 2017 at 10:52:49AM -0400, Brian Foster wrote:
> > > On Thu, Aug 10, 2017 at 03:09:09PM +0300, Alex Lyakas wrote:
> ...
> > > OTOH, just adding deferred ops buffer relogging might not be too much
> > > trouble either. ;) Anyways, thoughts?
> > 
> > I don't think it'd be difficult to add a _defer_bjoin operation that
> > maintains a list of buffers that we need to bhold across rolls.
> > 
> > I think xfs_buf->b_list is only used for delwri buffers, and a buffer
> > cannot be part of a transaction /and/ on a delwri list at the same time,
> > right?  So it shouldn't be hard to whip something up and couple this
> > patch to that.
> > 
> 
> Hmm.. so if a buffer is modified, logged, committed, put on the AIL and
> pushed, xfs_buf_item_push() locks it, puts it on the delwri queue and
> unlocks. At that point, I _think_ it may be possible for another thread
> to lock the buffer and join it to a new transaction. The delwri submit
> skips the buffer if it has become pinned or locked since the delwri
> queue (though I'm wondering if that unlocked pin check is racy against
> locked buffer modifications. I suppose that would require a full
> lock->pin->unlock cycle between the pin check and trylock however).

If it does race, we still catch pinned buffers in xfs_buf_submit() and
block there on them. SO a race is just sub-optimal behaviour, not a
bug.

> The question I have for buffer relogging is what's the best way to track
> the parts of the buffer that need to be relogged after a roll?
> Copy/translate the dirty (xfs_buf_log_format) segment map(s)?

Just mark it ordered?

That way it goes through the transaction commit, pinned and put into
the CIL and  gets moved forward in the AIL when the log checkpoints.
We don't need to relog the actual contents in this case, just ensure
it moves forward in the AIL appropriately while we hold it locked.
The ordered flag is removed at each commit the buffer goes through,
so as soon as we commit the final transaction it'll go back to
behaving like a normal buffer...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2017-08-11  2:22 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-09 11:06 [PATCH] xfs: hold xfs_buf locked between shortform->leaf conversion and the addition of an attribute alex
2017-08-09 13:17 ` Brian Foster
2017-08-09 21:33 ` Dave Chinner
2017-08-10  8:02   ` Alex Lyakas
2017-08-10 11:33     ` Dave Chinner
2017-08-10 12:09       ` Alex Lyakas
2017-08-10 14:52         ` Brian Foster
2017-08-10 17:55           ` Darrick J. Wong
2017-08-10 18:32             ` Brian Foster
2017-08-11  2:22               ` Dave Chinner [this message]
2017-08-11 14:27                 ` Brian Foster
2017-08-12  0:16                   ` Dave Chinner
2017-08-12 14:04                     ` Brian Foster
2017-08-14  0:28                       ` Dave Chinner
2017-08-14  8:11                         ` Alex Lyakas
2017-08-14 12:22                           ` Brian Foster
2017-08-14 16:04                             ` Alex Lyakas
2017-08-14 21:33                               ` Darrick J. Wong
2019-03-22  9:12                             ` Shyam Kaushik
2019-03-22 16:08                               ` Darrick J. Wong
2019-03-25 13:49                                 ` Shyam Kaushik
2019-03-25 18:17                                   ` Darrick J. Wong
2019-03-27 16:03                                     ` Alex Lyakas
2019-03-27 20:46                                       ` Dave Chinner
2019-03-28 11:26                                         ` Alex Lyakas
2017-08-17 20:38                         ` Brian Foster
2017-08-17 22:31                           ` Darrick J. Wong
2017-08-18 11:39                             ` Brian Foster
2017-08-18 15:37                               ` Darrick J. Wong
2017-08-18  2:04                           ` Dave Chinner
2017-08-18 11:42                             ` Brian Foster
2017-08-11  2:09             ` Dave Chinner
2017-08-11 14:30               ` Brian Foster
2017-08-11 12:53   ` Christoph Hellwig
2017-08-11 16:52     ` Darrick J. Wong
2017-08-12  7:37       ` Christoph Hellwig
2017-11-21 15:31 ` Libor Klepáč
2017-11-21 16:24   ` Brian Foster
2017-11-21 18:50     ` Darrick J. Wong
2017-11-30 17:55       ` Darrick J. Wong

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=20170811022204.GQ21024@dastard \
    --to=david@fromorbit.com \
    --cc=alex@zadarastorage.com \
    --cc=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=libor.klepac@bcom.cz \
    --cc=linux-xfs@vger.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).