From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755983AbbLAJcH (ORCPT ); Tue, 1 Dec 2015 04:32:07 -0500 Received: from mx2.suse.de ([195.135.220.15]:42659 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755565AbbLAJcC (ORCPT ); Tue, 1 Dec 2015 04:32:02 -0500 Subject: Re: [PATCH 1/4] loop: Enable correct physical blocksize To: Jeff Moyer References: <1447143190-44589-1-git-send-email-hare@suse.de> <1447143190-44589-2-git-send-email-hare@suse.de> Cc: Jens Axboe , Alexander Graf , Christoph Hellwig , Ming Lei , linux-kernel@vger.kernel.org From: Hannes Reinecke Message-ID: <565D6911.4010206@suse.de> Date: Tue, 1 Dec 2015 10:32:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/13/2015 09:57 PM, Jeff Moyer wrote: > Hi Hannes, > > Hannes Reinecke writes: > >> When running on files the physical blocksize is actually 4k, > > How did you come to that conclusion? Are you basing it on the file > system block size? If so, that's configurable at mkfs time and can be > anything from 512 bytes to 64k on current in-tree file systems that I > know of (depending on platform, of course). > loop.c does this (in do_loop_switch()): mapping_set_gfp_mask(old_file->f_mapping, lo->old_gfp_mask); lo->lo_backing_file = file; lo->lo_blocksize = S_ISBLK(mapping->host->i_mode) ? mapping->host->i_bdev->bd_block_size : PAGE_SIZE; lo->old_gfp_mask = mapping_gfp_mask(mapping); So either it's a block device, then we're taking the blocksize of the underlying device, or we're using PAGE_SIZE. Which is architecture dependent, of course. > The main use for physical block size, as I understand it, is to allow > partitioning utilities to place partitions on physical block boundaries > of the underlying storage. The benefit of that is to avoid > read-modify-writes for I/O which is naturally sized and aligned. If we > carry that forward to loop, then I think it does makes sense to key off > of the file system block size, but the fact remains that 4k is not > universal. > The main point here is that some utilities (eg bootloaders) need to know the _physical_ location of a particular blob, for which it needs to know the physical blocksize. > So, I think the idea is sound, but you should be setting the physical > block size to sb->s_blocksize. And I don't see any reason why we > wouldn't do this by default, do you? > Neither do I. But the code doesn't treat it that way, so I elected to stay with the current version. > If you end up reposting this patch, would you mind including more of > this rationale in your commit message? > Sure. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)