On Wed, Sep 06, 2017 at 08:06:31AM +1000, Dave Chinner wrote: > On Tue, Sep 05, 2017 at 09:00:26AM -0600, Jens Axboe wrote: > > On 09/05/2017 01:37 AM, Chandan Rajendra wrote: > > > On Tuesday, September 5, 2017 12:12:08 PM IST Omar Sandoval wrote: > > >> On Mon, Sep 04, 2017 at 11:31:39PM -0700, Christoph Hellwig wrote: > > >>> On Tue, Sep 05, 2017 at 11:14:42AM +0530, Chandan Rajendra wrote: > > >>>> Linux kernel commit 6c6b6f28b3335fd85ec833ee0005d9c9dca6c003 (loop: set > > >>>> physical block size to PAGE_SIZE) now sets PAGE_SIZE as the default > > >>>> physical sector size of loop devices. On ppc64, this causes loop devices > > >>>> to have 64k as the physical sector size. > > >>> > > >>> Eek. We'll need to revert the loop change ASAP! > > >> > > >> Most annoying patch series ever. It hasn't made it to Linus' tree yet, > > >> right? We can revert (although the later change depends on that), fold > > >> in a fix, or apply a fix on top of it, whatever Jens prefers. > > >> > > >> > > > > > > My bad. 6c6b6f28b3335fd85ec833ee0005d9c9dca6c003 is the commit id from > > > Linux-next. I don't see this commit in Linus's git tree. > > > > Right, it's only queued up, and scheduled for the 2nd part of the > > block changes for 4.14. It should have been PAGE_CACHE_SIZE, but > > we don't have that anymore... > > But PAGE_CACHE_SIZE was equal to PAGE_SIZE, so that would have been > wrong, too. > > I just don't see why this is necessary, given that buffered IO > through the upper filesystem will already be doing page > sized/aligned IO where possible (because that's what the page cache > does!). And for direct IO the loop device should just export the > underlying host filesystem logical/physical sector sizes, which > should be optimal for the backing storage to begin with. > > Someone want to enlighten me as to what problem is being solved > here? I already sent a patch to fix this: https://marc.info/?l=linux-block&m=150464670510301&w=2 Loop was using the default physical block size of 512, which is a lie according to the definition of the physical block size: /** * blk_queue_physical_block_size - set physical block size for the queue * @q: the request queue for the device * @size: the physical block size, in bytes * * Description: * This should be set to the lowest possible sector size that the * hardware can operate on without reverting to read-modify-write * operations. */ Clearly for buffered loop devices, this unit is a page. The change was pedantic, so whatever, my patch above makes things backwards compatible.
