From: Filipe Manana <fdmanana@kernel.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 2/3] btrfs: don't fallback to buffered IO for NOWAIT direct IO writes
Date: Mon, 4 Jul 2022 13:19:36 +0100 [thread overview]
Message-ID: <20220704121936.GA656399@falcondesktop> (raw)
In-Reply-To: <YsLZLBPM4KkvpN5v@infradead.org>
On Mon, Jul 04, 2022 at 05:12:28AM -0700, Christoph Hellwig wrote:
> On Mon, Jul 04, 2022 at 01:11:14PM +0100, Filipe Manana wrote:
> > We generally don't mind about going beyond 80 characters as long as it's not too
> > much (83 here is reasonable), given that the new limit was changed to 100 characters
> > some time ago. And this applies to comments and code.
>
> The limit is for individual lines where that benefits readability.
> That's never the case for block comments by definition.
First time I'm hearing it, and never had complaints before.
>
> > > > + * and may block there for many reasons (reserving space for example).
> > > > + */
> > > > + if (iocb->ki_flags & IOCB_NOWAIT) {
> > > > + err = -EAGAIN;
> > > > + goto out;
> > > > + }
> > >
> > > but more importantly, shouldn't this be above the buffered label? The
> > > only places that jumps to it is the alignment check, and if the
> > > alignment is incorrect now, it won't get any better in a workqueue
> > > context.
> >
> > It was intentionally placed there not only because of that single goto but
> > to make it less error prone in case we get more gotos into that label in
> > the future.
>
> But it does the wrong thing for the only use of the goto right now..
Why is it wrong? The purpose it to never fallback directly to buffered IO if
it's a NOWAIT write. Moving the check to above the label, would make the
non-aligned case fallback directly to buffered IO under NOWAIT.
next prev parent reply other threads:[~2022-07-04 12:20 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-04 11:42 [PATCH 0/3] btrfs: a few direct IO fixes/improvements fdmanana
2022-07-04 11:42 ` [PATCH 1/3] btrfs: return -EAGAIN for NOWAIT dio reads/writes on compressed and inline extents fdmanana
2022-07-04 11:57 ` Christoph Hellwig
2022-07-07 16:47 ` David Sterba
2022-07-04 11:42 ` [PATCH 2/3] btrfs: don't fallback to buffered IO for NOWAIT direct IO writes fdmanana
2022-07-04 12:00 ` Christoph Hellwig
2022-07-04 12:11 ` Filipe Manana
2022-07-04 12:12 ` Christoph Hellwig
2022-07-04 12:19 ` Filipe Manana [this message]
2022-07-04 12:37 ` Christoph Hellwig
2022-07-04 11:42 ` [PATCH 3/3] btrfs: fault in pages for dio reads/writes in a more controlled way fdmanana
2022-07-08 15:20 ` [PATCH 0/3] btrfs: a few direct IO fixes/improvements David Sterba
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=20220704121936.GA656399@falcondesktop \
--to=fdmanana@kernel.org \
--cc=hch@infradead.org \
--cc=linux-btrfs@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).