Re: [Evms-devel] Re: [PATCH] EVMS core 4/4: evms_biosplit.h

Re: [Evms-devel] Re: [PATCH] EVMS core 4/4: evms_biosplit.h

[Mark Peloquin]

On 10/03/2002 at 10:05 AM, Christoph Hellwig wrote:

>> +static mempool_t *my_bio_split_pool, *my_bio_pool;
>> +static kmem_cache_t *my_bio_split_slab, *my_bio_pool_slab;

> Umm, static variables in header files?

Yupp, that wasn't accidental.

Based on the conclusions mutually agreed on in the OLS
BOF on bio splitting, it was decided that each driver
having the need to split bios *should* have their own
private bio pools to use exclusively for this purpose.
Thus any plugin that includes this header gets their
own private copies of these variables.

>> +
>> +/**
>> + * slab_pool_alloc
>> + * @gfp_mask: GFP allocation flag
>> + * @data:     mempool prototype required fields
>> + *
>> + * mempool allocate function
>> + **/
>> +static void *
>> +slab_pool_alloc(int gfp_mask, void *data)
>> +{
>> +  return kmem_cache_alloc(data, gfp_mask);
>> +}
>> +
>> +/**
>> + * slab_pool_free
>> + * @ptr:      mempool prototype required fields
>> + * @data:     mempool prototype required fields
>> + *
>> + * mempool free function
>> + **/
>> +static void
>> +slab_pool_free(void *ptr, void *data)
>> +{
>> +  kmem_cache_free(data, ptr);
>> +}

> I think these two could go to slab.c instead.

I agree. I've seen a few other static versions of the
same thing floating around.

>> +  if (!my_bio_split_slab) {
>> +        panic("unable to create EVMS Bio Split cache.");

> What about graceful error handling?

Alan already questioned this as well... and I agree. I've
already made the necessary changes.

> All in all I think this should rather be a source file, I can't
> see anything EVMS-specific either.

There may likely be generic code to perform bio splitting
at some (hopefully soon) point in time. I'm not even going
to assume that what EVMS has done will or should be it.
This support was a temporary implementation that does
allow several plugins in EVMS to work correctly with the
large I/Os now available in 2.5. I do agree, what ever is
done should be placed in some commonly usable location.

Mark

Re: [Evms-devel] Re: [PATCH] EVMS core 4/4: evms_biosplit.h

[Christoph Hellwig]

On Thu, Oct 03, 2002 at 03:36:24PM -0500, Mark Peloquin wrote:
> 
> On 10/03/2002 at 10:05 AM, Christoph Hellwig wrote:
> 
> >> +static mempool_t *my_bio_split_pool, *my_bio_pool;
> >> +static kmem_cache_t *my_bio_split_slab, *my_bio_pool_slab;
> 
> > Umm, static variables in header files?
> 
> Yupp, that wasn't accidental.
> 
> Based on the conclusions mutually agreed on in the OLS
> BOF on bio splitting, it was decided that each driver
> having the need to split bios *should* have their own
> private bio pools to use exclusively for this purpose.
> Thus any plugin that includes this header gets their
> own private copies of these variables.

Having variables and (non-inline) functions is very bad style and a
perfect method for the obsfucated C contest.  Just let every module
that needs bio-splitting declare it's own variables.

Re: [Evms-devel] Re: [PATCH] EVMS core 4/4: evms_biosplit.h

[Mark Peloquin]

On 10/03/02 at 8:57 AM, Alan Cox wrote:
> > +static void
> > +bio_split_setup(char * split_name, char * bio_name)
> > +{
> > + /* initialize MY bio split record pool */
> > + my_bio_split_slab = kmem_cache_create(split_name,
> > +                               sizeof
> > +                               (struct bio_split_cb),
> > +                               0, SLAB_HWCACHE_ALIGN,
> > +                                                NULL, NULL);
> > + if (!my_bio_split_slab) {
> > +       panic("unable to create EVMS Bio Split cache.");

> If IBM are going to be doing telco grade stuff you could start now by
> failing politely in this case 8)

Your absolutely right! The appropriate changes will be made
immediately. Also, please bear in mind that this bio
splitting code is just temporary until the community comes
up with an acceptable generic solution.

Mark





-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Evms-devel mailing list
Evms-devel@lists.sourceforge.net
To subscribe/unsubscribe, please visit:
 https://lists.sourceforge.net/lists/listinfo/evms-devel