All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: Damien Le Moal <damien.lemoal@wdc.com>
Cc: linux-scsi@vger.kernel.org,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	"James E . J . Bottomley" <James.Bottomley@HansenPartnership.com>,
	Sathya Prakash <sathya.prakash@broadcom.com>,
	Chaitra P B <chaitra.basappa@broadcom.com>,
	Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>,
	MPT-FusionLinux.pdl@broadcom.com, Hannes Reinecke <hare@suse.de>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v5] sd: Check for unaligned partial completion
Date: Mon, 20 Feb 2017 21:35:08 -0500	[thread overview]
Message-ID: <yq11sus5l0j.fsf@oracle.com> (raw)
In-Reply-To: <20170217000130.22471-1-damien.lemoal@wdc.com> (Damien Le Moal's message of "Fri, 17 Feb 2017 09:01:30 +0900")

>>>>> "Damien" == Damien Le Moal <damien.lemoal@wdc.com> writes:

Hi Damien,

Damien> Move the partial completion alignement check of mpt3sas to a
Damien> generic implementation in sd_done so that the check ignores
Damien> REQ_TYPE_FS requests with special payload size handling
Damien> (REQ_OP_DISCARD, REQ_OP_WRITE_SAME, REQ_OP_ZONE_REPORT and
Damien> REQ_OP_ZONE_RESET). For the remaining REQ_OP_FLUSH, REQ_OP_READ
Damien> and REQ_OP_WRITE, we only need to check the resid alignment,
Damien> correct it if necessary and then correct good_bytes. Note that
Damien> in this case, good_bytes will always initially be 0 or aligned
Damien> on the device logical block size, so correcting resid alignment
Damien> will always result in good_bytes also being properly aligned.

I'm still not keen on having two orthogonal sanity checks wrt. figuring
out how much of a request has been completed.

Also, I find your approach hard to follow in the case where
sd_completed_bytes() is called after the resid has been adjusted. It
works, but it's not immediately obvious that that's the case. Which to
me is an indication that this entire thing needs a thorough cleanup.

If you don't feel like mucking more with this, I understand. In that
case might pick up your patch and attempt to clean things up later.

-- 
Martin K. Petersen	Oracle Linux Engineering

  parent reply	other threads:[~2017-02-21  2:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-17  0:01 [PATCH v5] sd: Check for unaligned partial completion Damien Le Moal
2017-02-17  8:25 ` Christoph Hellwig
2017-02-20  0:05   ` Damien Le Moal
2017-02-20 17:34 ` Bart Van Assche
2017-02-21  1:44   ` Damien Le Moal
2017-02-21  2:35 ` Martin K. Petersen [this message]
2017-02-21  3:45   ` Damien Le Moal
2017-02-21  4:01     ` Martin K. Petersen
2017-02-21  4:21   ` Bart Van Assche
2017-02-21  7:47     ` Damien Le Moal
2017-02-22  4:24       ` Martin K. Petersen
2017-02-22  5:01         ` Damien Le Moal
2017-02-23  4:44         ` Damien Le Moal
2017-02-23  4:53           ` Damien Le Moal

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=yq11sus5l0j.fsf@oracle.com \
    --to=martin.petersen@oracle.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=MPT-FusionLinux.pdl@broadcom.com \
    --cc=chaitra.basappa@broadcom.com \
    --cc=damien.lemoal@wdc.com \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sathya.prakash@broadcom.com \
    --cc=suganath-prabu.subramani@broadcom.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.