Re: [PATCH 1/4] scatterlist: new helper functions

From: FUJITA Tomonori
Date: Tue Mar 15 2011 - 22:36:26 EST


On Tue, 15 Mar 2011 17:44:59 -0700
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:

> > > This restriction is due to hardware specification or the software
> > > design (e.g. memstick layer)? If it is due to the latter, why can't
> > > you fix that?
> >
> > Yes.
> > I already tried addressing some shortcomings of memstick layer, no no, I
> > don't want to deal with its author, Alex Dubov again.
> > I think this code tries to be too clever/complex for the range of
> > devices/speeds it supports, but I rather leave it as is.
> >
>
> I have to say, these aren't very good reasons for a particular
> implementation!

Agreed. We need to fix it.


> > To be honest, the code in question is for >5 year old memstick standard
> > cards, thats hardly anybody uses.
> > It works, it is more or less simple, its not performance bound, its
> > testd, and thus I want to keep it as is _for_ now.
> >
> >
> > Why I break sg lists into chunks?
> > Because unlike vast majority of block devices, I need to do FTL in the
> > driver, thus its easier to work on eraseblock boundary.
> > Also unlike anything else, you can't just read/write a sector from a
> > memorystick (especially the legacy one), you have to perform full dance
> > of commands.
> >
> > Not to mention error handling (like if you failed to write to block, you
> > must try to choose another one, etc...)
> >
> > (Of course writes follow same rules as raw nand flash, thats is writes
> > only clear bits, and you can erase a eraseblock only).
>
> hm. If you think there's little likelihood that other drivers will
> need the new sg functions in the future then perhaps they should be
> made private to the memstick driver, rather than bloating everyone's
> kernels. Which is, I think, the exact opposite of what I suggested
> last year :(
>
> Fujita-san, you've gone all quiet. Do you believe that these functions
> should be added to the sg API?

I don't think so.

Splitting (or merging) a request and playing with sg lists inside a
driver is a bad idea. Such should be done in the block layer.

I still don't see why the block layer can't do that for the driver.

I believe that the helper functions for such should not be added.
--
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/