From: Linus Torvalds <torvalds@linux-foundation.org>
To: Jens Axboe <jaxboe@fusionio.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] One important block fix for 2.6.36-rc
Date: Sat, 25 Sep 2010 09:59:28 -0700 [thread overview]
Message-ID: <AANLkTikcR8fu11jyLcD3Fuvm4TPxU=46jLcpsYryHONP@mail.gmail.com> (raw)
In-Reply-To: <4C9DD2D5.8090807@fusionio.com>
On Sat, Sep 25, 2010 at 3:45 AM, Jens Axboe <jaxboe@fusionio.com> wrote:
>
> Please pull asap, thanks.
Pulled, but I _really_ wish you started looking at cleanliness issues,
even with bug-reports.
That's a singularly stupid and unreadable way of testing "do those
flags match". It is quite possible that the compiler fixes up the
stupidity, but that doesn't make it much better.
Here's how things like this _should_ be tested:
#define REQ_FLAGS_MUST_MATCH (REQ_SECURE | REQ_DISCARD)
...
/* Check that flags match in the required bits */
if ((req->cmd_flags ^ next->cmd_flags) & REQ_FLAGS_MUST_MATCH)
return 0;
...
which is (a) smaller (b) easier and clearer to add flags as needed and
(c) actually more readable due to not having duplicated logic (not
just one if-statement, but one mask too).
And yes, I think you'd want to add a few flags there. Looking at the
REQ_xyz flags, I suspect most o them should really really match for
requests to be mergeable. Wouldn't it be better to think of it in
terms of "do we really allow different cmd_flags requests to merge"
than adding one bit ad-hoc at a time?
Linus
next prev parent reply other threads:[~2010-09-25 17:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-25 10:45 [GIT PULL] One important block fix for 2.6.36-rc Jens Axboe
2010-09-25 16:59 ` Linus Torvalds [this message]
2010-09-26 3:21 ` Jens Axboe
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='AANLkTikcR8fu11jyLcD3Fuvm4TPxU=46jLcpsYryHONP@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=jaxboe@fusionio.com \
--cc=linux-kernel@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).