linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Tejun Heo <htejun@gmail.com>
Cc: linux-scsi <linux-scsi@vger.kernel.org>,
	linux-ide <linux-ide@vger.kernel.org>,
	Jens Axboe <Jens.Axboe@oracle.com>,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>,
	Jeff Garzik <jeff@garzik.org>
Subject: Re: [PATCH] libata: eliminate the home grown dma padding in favour of	that provided by the block layer
Date: Tue, 08 Jan 2008 22:24:32 -0600	[thread overview]
Message-ID: <1199852673.3534.76.camel@localhost.localdomain> (raw)
In-Reply-To: <47842D29.8020509@gmail.com>


On Wed, 2008-01-09 at 11:10 +0900, Tejun Heo wrote:
> James Bottomley wrote:
> > ATA requires that all DMA transfers begin and end on word boundaries.
> > Because of this, a large amount of machinery grew up in ide to adjust
> > scatterlists on this basis.  However, as of 2.5, the block layer has a
> > dma_alignment variable which ensures both the beginning and length of a
> > DMA transfer are aligned on the dma_alignment boundary.  Although the
> > block layer does adjust the beginning of the transfer to ensure this
> > happens, it doesn't actually adjust the length, it merely makes sure
> > that space is allocated for transfers beyond the declared length.  The
> > upshot of this is that scatterlists may be padded to any size between
> > the actual length and the length adjusted to the dma_alignment safely
> > knowing that memory is allocated in this region.
> > 
> > Right at the moment, SCSI takes the default dma_aligment which is on a
> > 512 byte boundary.  Note that this aligment only applies to transfers
> > coming in from user space.  However, since all kernel allocations are
> > automatically aligned on a minimum of 32 byte boundaries, it is safe to
> > adjust them in this manner as well.
> > 
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > 
> > ---
> > 
> > This also fixes a current panic in libsas with SATAPI devices.  I was
> > trying to trace a bad interaction between the libata padding and the new
> > sglist code which was the root cause, and ended up concluding that the
> > whole libata padding thing was redundant.
> > 
> > In a future improvement, I think we can relax SCSI dma_alignemnt (it's
> > causing almost every user space command to be copied instead of directly
> > passed through) and allow devices and transports to build up their own
> > requirements instead.
> 
> Great but this intersects with currently queued and pending changes for
> libata-dev#upstream and needs to be merged. 

That's OK, I can do the merge .. that's what git is for.

>  Also, DMA alignment at
> block layer isn't enough for ATA.  ATA needs drain buffers for ATAPI
> commands with variable length response.  :-(

OK, where is this in the libata code?  The dma_pad size is only 4 bytes,
so this drain, I assume is only a word long?  Given the word alignment
requirements of ATA doesn't this still mean it's only draining up to the
word boundary anyway (so the code is still correct)?

James



  reply	other threads:[~2008-01-09  4:24 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-31 21:56 [PATCH] libata: eliminate the home grown dma padding in favour of that provided by the block layer James Bottomley
2007-12-31 22:56 ` Jeff Garzik
2008-01-03  7:58 ` FUJITA Tomonori
2008-01-03 15:12   ` James Bottomley
2008-01-09  2:10 ` Tejun Heo
2008-01-09  4:24   ` James Bottomley [this message]
2008-01-09  5:13     ` Tejun Heo
2008-01-09 15:13       ` James Bottomley
2008-01-18 23:14 ` [PATCH RESEND] " James Bottomley
2008-02-01 19:40   ` [PATCH RESEND number 2] " James Bottomley
2008-02-01 20:02     ` Jeff Garzik
2008-02-01 21:09       ` James Bottomley
2008-02-03  3:04         ` Tejun Heo
2008-02-03  4:32           ` James Bottomley
2008-02-03  7:37             ` Tejun Heo
2008-02-03 14:38               ` James Bottomley
2008-02-03 15:14                 ` Tejun Heo
2008-02-03 16:12                   ` James Bottomley
2008-02-03 16:38                     ` Jeff Garzik
2008-02-03 17:12                       ` James Bottomley
2008-02-04  1:21                         ` Tejun Heo
2008-02-04  1:28                     ` Tejun Heo
2008-02-04  9:25                       ` Tejun Heo
2008-02-04 14:43                         ` Tejun Heo
2008-02-04 16:23                           ` James Bottomley
2008-02-05  0:06                             ` Tejun Heo
2008-02-05  0:32                               ` James Bottomley
2008-02-05  0:43                                 ` Tejun Heo
2008-02-05  0:53                                   ` James Bottomley
2008-02-05  1:07                                     ` Tejun Heo
2008-02-05  5:03                                       ` James Bottomley
2008-02-05  5:22                                         ` Tejun Heo
2008-02-04 15:43                         ` James Bottomley

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=1199852673.3534.76.camel@localhost.localdomain \
    --to=james.bottomley@hansenpartnership.com \
    --cc=Jens.Axboe@oracle.com \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=htejun@gmail.com \
    --cc=jeff@garzik.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-scsi@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).