From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Allison Collins <allison.henderson@oracle.com>
Cc: Christoph Hellwig <hch@infradead.org>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH v5 04/14] xfs: Add xfs_has_attr and subroutines
Date: Tue, 21 Jan 2020 14:30:59 -0800 [thread overview]
Message-ID: <20200121223059.GG8247@magnolia> (raw)
In-Reply-To: <2b29c0a0-03bb-8a21-8a8a-fd4754bff3ff@oracle.com>
On Tue, Dec 24, 2019 at 09:21:49PM -0700, Allison Collins wrote:
> On 12/24/19 5:18 AM, Christoph Hellwig wrote:
> > On Wed, Dec 11, 2019 at 09:15:03PM -0700, Allison Collins wrote:
> > > From: Allison Henderson <allison.henderson@oracle.com>
> > >
> > > This patch adds a new functions to check for the existence of
> > > an attribute. Subroutines are also added to handle the cases
> > > of leaf blocks, nodes or shortform. Common code that appears
> > > in existing attr add and remove functions have been factored
> > > out to help reduce the appearance of duplicated code. We will
> > > need these routines later for delayed attributes since delayed
> > > operations cannot return error codes.
> >
> > Can you explain why we need the ahead of time check? The first
> > operation should be able to still return an error, and doing
> > a separate check instead of letting the actual operation fail
> > gracefully is more expensive, and also creates a lot of additional
> > code. As is I can't say I like the direction at all.
> >
>
> This one I can answer quickly: later when we get into delayed attributes,
> this will get called from xfs_defer_finish_noroll as part of a .finish_item
> call back. If these callbacks return anything other than 0 or -EAGAIN, it
> causes a shutdown.
When does this happen, exactly? Are you saying that during log
recovery, we can end up replaying a delayed attr log item that hits
ENOATTR/EEXIST somewhere and passes that out, which causes log recovery
to fail?
> Which is not what we would want for example: when the
> user tries to rename a non-existent attribute. The error code needs to go
> back up. So we check for things like that before starting a delayed
> operation. Hope that helps. Thanks!
...because as far as requests from user programs goes, we should be
doing all these precondition checks after allocating a transaction and
ILOCKing the inode, so that we can send the error code back to userspace
without cancelling a dirty transaction.
(I dunno, am I misunderstanding here?)
> Allison
next prev parent reply other threads:[~2020-01-21 22:31 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-12 4:14 [PATCH v5 00/14] xfs: Delay Ready Attributes Allison Collins
2019-12-12 4:15 ` [PATCH v5 01/14] xfs: Remove all strlen in all xfs_attr_* functions for attr names Allison Collins
2019-12-12 4:15 ` [PATCH v5 02/14] xfs: Replace attribute parameters with struct xfs_name Allison Collins
2019-12-13 13:07 ` Brian Foster
2019-12-14 6:57 ` Allison Collins
2019-12-12 4:15 ` [PATCH v5 03/14] xfs: Embed struct xfs_name in xfs_da_args Allison Collins
2019-12-12 4:15 ` [PATCH v5 04/14] xfs: Add xfs_has_attr and subroutines Allison Collins
2019-12-13 13:08 ` Brian Foster
2019-12-14 6:58 ` Allison Collins
2019-12-24 12:18 ` Christoph Hellwig
2019-12-25 4:21 ` Allison Collins
2020-01-21 22:30 ` Darrick J. Wong [this message]
2020-01-22 0:25 ` Allison Collins
2020-01-25 16:27 ` Allison Collins
2020-01-25 23:08 ` Christoph Hellwig
2020-01-26 4:03 ` Allison Collins
2019-12-12 4:15 ` [PATCH v5 05/14] xfs: Factor out new helper functions xfs_attr_rmtval_set Allison Collins
2019-12-24 12:14 ` Christoph Hellwig
2019-12-25 17:43 ` Allison Collins
2020-01-06 14:46 ` Brian Foster
2020-01-06 18:29 ` Allison Collins
2020-01-06 21:45 ` Darrick J. Wong
2020-01-06 23:33 ` Dave Chinner
2019-12-12 4:15 ` [PATCH v5 06/14] xfs: Factor up trans handling in xfs_attr3_leaf_flipflags Allison Collins
2019-12-24 12:16 ` Christoph Hellwig
2019-12-25 20:49 ` Allison Collins
2019-12-12 4:15 ` [PATCH v5 07/14] xfs: Factor out xfs_attr_leaf_addname helper Allison Collins
2019-12-13 14:15 ` Brian Foster
2019-12-14 6:58 ` Allison Collins
2019-12-24 12:22 ` Christoph Hellwig
2019-12-26 17:04 ` Allison Collins
2019-12-12 4:15 ` [PATCH v5 08/14] xfs: Factor up xfs_attr_try_sf_addname Allison Collins
2019-12-13 14:15 ` Brian Foster
2019-12-14 6:58 ` Allison Collins
2019-12-24 12:25 ` Christoph Hellwig
2019-12-27 3:23 ` Allison Collins
2019-12-12 4:15 ` [PATCH v5 09/14] xfs: Factor up trans roll from xfs_attr3_leaf_setflag Allison Collins
2019-12-24 12:26 ` Christoph Hellwig
2019-12-12 4:15 ` [PATCH v5 10/14] xfs: Factor out xfs_attr_rmtval_invalidate Allison Collins
2019-12-12 4:15 ` [PATCH v5 11/14] xfs: Factor up trans roll in xfs_attr3_leaf_clearflag Allison Collins
2019-12-24 12:28 ` Christoph Hellwig
2019-12-12 4:15 ` [PATCH v5 12/14] xfs: Check for -ENOATTR or -EEXIST Allison Collins
2019-12-13 14:16 ` Brian Foster
2019-12-14 7:56 ` Allison Collins
2019-12-12 4:15 ` [PATCH v5 13/14] xfs: Add delay ready attr remove routines Allison Collins
2019-12-13 17:30 ` Brian Foster
2019-12-14 19:21 ` Allison Collins
2019-12-24 12:30 ` Christoph Hellwig
2019-12-24 23:18 ` Allison Collins
2019-12-12 4:15 ` [PATCH v5 14/14] xfs: Add delay ready attr set routines Allison Collins
2019-12-24 12:02 ` [PATCH v5 00/14] xfs: Delay Ready Attributes Christoph Hellwig
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=20200121223059.GG8247@magnolia \
--to=darrick.wong@oracle.com \
--cc=allison.henderson@oracle.com \
--cc=hch@infradead.org \
--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).