Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1635614
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] ata-sff: always map page before data transfer |
| Date | 2017-05-04 12:00 +0200 |
| Message-ID | <tDlIe-1IX-11@gated-at.bofh.it> (permalink) |
| References | <tCJ0d-ID-5@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
> I don't understand all the factors at play here, so thoughts are definitely > welcome. I don't fully understand the old code either. One thing that is weird is the "use a bounce buffer comment" which doesn't make any sense. The other is the local_irq_save, which isn't really needed for kmap_atomic to start with, but maybe that's the reason why the original author didn't want to do it unconditionally? So based on that: > + /* FIXME: use a bounce buffer */ drop this comment.. > + local_irq_save(flags); .. remove the local_irq_save/local_irq_restore .. > + /* do the actual data transfer */ > + ap->ops->sff_data_xfer(qc, buf + offset, qc->sect_size, > + do_write); .. and a nice alittle cleanup move the do_write onto the previous line. > + /* FIXME: use bounce buffer */ > + local_irq_save(flags); > + buf = kmap_atomic(page); > > + /* do the actual data transfer */ > + consumed = ap->ops->sff_data_xfer(qc, buf + offset, > + count, rw); And same here. And we should be fine.
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] ata-sff: always map page before data transfer Tycho Andersen <tycho@docker.com> - 2017-05-02 18:40 +0200
Re: [PATCH] ata-sff: always map page before data transfer Christoph Hellwig <hch@infradead.org> - 2017-05-04 12:00 +0200
Re: [PATCH] ata-sff: always map page before data transfer Tycho Andersen <tycho@docker.com> - 2017-05-05 00:20 +0200
csiph-web