Re: [PATCH 3/5] brd: Add getgeo to block ops

From: Boaz Harrosh
Date: Thu Aug 28 2014 - 11:43:57 EST


On 08/28/2014 06:11 PM, Matthew Wilcox wrote:
> On Thu, Aug 28, 2014 at 10:26:31AM +0300, Boaz Harrosh wrote:
<>
>> No with patch 5/5 the 4k stuff is good.
>>
>> What I'm saying is that with (64, 32, x) fdisk offers a very high first
>> sector and with all 1(s) it will allow a low value like 4k
>>
>> For example with (64, 32, x) + the 4k patch in 5/5 with a 4M brd disk it
>> will offer 40 (20K) as first possible sector.
>>
>> With this patch applied it will offer 8 (4K) as first sector, which is what
>> I want
>
> That makes for a much better changelog entry than what you wrote above :-)
>

OK Will do thanks

>> But is it wrong? why?
>>
>> I guess that until now no one cared about wasted space at the beginning
>> but with brd I do. Your words this is all "fake" then why at all.
>> With all 1(s) the CHS convoluted math just disappears and gets out of the
>> way.
>
> It is all fake. Nobody's reported their real CHS geometry in twenty
> years. A drive's geometry has been more complex than a fixed number
> of sectors per track, and anyway the hd_geometry struct tops out at
> describing a 500GB drive (with 63 heads and 255 sectors).
>

Right?

> My concern is that clearly (64, 32, x) works since other drivers are
> doing it. (1, 1, 1) is stepping into the unknown, and we don't know
> what applications are going to make of this value.
>

But you are ignoring my "it does not work" complain above?

I have tested with fdisk cfdisk gparted (and parted) and they all do
the same as before or better in the case of fdisk. I have reviewed the
source code to fdisk, and was satisfied that 1,1,1 is actually good and
fixes my problem above.

Why can't we put this in Linux-next for a while and let imaginary
"applications are going to make of this value" complain in do time.

Does anyone know of any application other then above list that I should test
with?

Let me remind you that partitions was completely broken on brd since its
inception, and no one knew about it until I started testing, so its not
like we can do *any kind* of harm with this.

> This really needs to be something that's handled by the block midlayer.
> Forcing drivers to make this stuff up is only leading to pain and
> suffering. Drivers that don't have a getgeo method should be given a
> default geometry that makes all known users of getgeo do the right thing.
>

Fine, I promise to do this once this test driver is to run for a cycle or
two I will do it. I will clean the all stack and have two defaults for
faking drivers and let be those old drivers that actually go to hardware
to read it. Let us have this brd driver as a test bed and I promise to
do this.

This is regular Kernel procedure. I have audited and tested to the best of
my, or ML readers ability, and we have in place a staging procedure that should
catch any problems soon enough.

Since when have we become so defensive on ZERO risk code in the Kernel. We are
changing core and revolutionary systems every day for breakfast? With far more
out reaching implications. And problems come up and we fix them.

Can we move forward please
Boaz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/