[PATCHSET 00/18] open-osd: OSD Initiator library for Linux - Kernel

This is a discussion on [PATCHSET 00/18] open-osd: OSD Initiator library for Linux - Kernel ; On Tue, 4 Nov 2008 18:44:06 +0200 Boaz Harrosh wrote: > Headers only patch. > > osd_protocol.h > Contains a C-fied definition of the T10 OSD standard > osd_types.h > Contains CPU order common used types > osd_initiator.h > API ...

+ Reply to Thread
Page 2 of 3 FirstFirst 1 2 3 LastLast
Results 21 to 40 of 53

Thread: [PATCHSET 00/18] open-osd: OSD Initiator library for Linux

  1. Re: [PATCH 03/18] libosd: OSDv1 Headers

    On Tue, 4 Nov 2008 18:44:06 +0200
    Boaz Harrosh wrote:

    > Headers only patch.
    >
    > osd_protocol.h
    > Contains a C-fied definition of the T10 OSD standard
    > osd_types.h
    > Contains CPU order common used types
    > osd_initiator.h
    > API definition of the osd_initiator library
    > osd_sec.h
    > Contains High level API for the security manager.
    >
    > [Note that checkpatch spews errors on things that are valid in this context
    > and will not be fixed]
    >
    > --- /dev/null
    > +++ b/include/scsi/osd_initiator.h


    This header uses quite a few things without including the header files
    which define them. That's a bit risky - it causes compile breakage
    across architectures, across config changes and across time.

    > @@ -0,0 +1,332 @@
    > +/*
    > + * osd_initiator.h - OSD initiator API definition
    > + *
    > + * Copyright (C) 2008 Panasas Inc. All rights reserved.
    > + *
    > + * Authors:
    > + * Boaz Harrosh
    > + * Benny Halevy
    > + *
    > + * This program is free software; you can redistribute it and/or modify
    > + * it under the terms of the GNU General Public License version 2
    > + *
    > + */
    > +#ifndef __OSD_INITIATOR_H__
    > +#define __OSD_INITIATOR_H__
    > +
    > +#include
    > +
    > +#include "osd_protocol.h"
    > +#include "osd_types.h"
    > +
    > +/* Note: "NI" in comments below means "Not Implemented yet" */
    > +
    > +/*
    > + * Object-based Storage Device.
    > + * This object represents an OSD device.
    > + * It is not a full linux device in any way. It is only
    > + * a place to hang resources associated with a Linux
    > + * request Q and some default properties.
    > + */
    > +struct osd_dev {
    > + struct scsi_device *scsi_dev;


    scsi_device

    > + unsigned def_timeout;
    > +};
    > +
    > +void osd_dev_init(struct osd_dev *, struct scsi_device *scsi_dev);
    > +void osd_dev_fini(struct osd_dev *);
    > +
    > +struct osd_request;
    > +typedef void (osd_req_done_fn)(struct osd_request *, void *);
    > +
    > +struct osd_request {
    > + struct osd_cdb cdb;
    > + struct osd_data_out_integrity_info out_data_integ;
    > + struct osd_data_in_integrity_info in_data_integ;
    > +
    > + struct osd_dev *osd_dev;
    > + struct request *request;
    > +
    > + struct _osd_req_data_segment {
    > + void *buff;
    > + unsigned alloc_size; /* 0 here means not allocated by us */
    > + unsigned total_bytes;
    > + } set_attr, enc_get_attr, get_attr;
    > +
    > + struct _osd_io_info {
    > + struct bio *bio;
    > + u64 total_bytes;


    u64(!)

    > + struct request *req;
    > + struct _osd_req_data_segment *last_seg;
    > + u8 *pad_buff;
    > + } out, in;
    > +
    > + gfp_t alloc_flags;


    gfp_t

    > + unsigned timeout;
    > + unsigned retries;
    > + u8 sense[OSD_MAX_SENSE_LEN];
    > + enum osd_attributes_mode attributes_mode;
    > +
    > + osd_req_done_fn *async_done;
    > + void *async_private;
    > + int async_error;
    > +};


    etc, etc, etc. Please review all that.

    > +struct osd_request *osd_start_request(struct osd_dev *, gfp_t gfp);
    > +int osd_finalize_request(struct osd_request *or,
    > + u8 options, const void *cap, const u8 *cap_key);
    > +void osd_req_set_master_seed_xchg(struct osd_request *, ...);/* NI */
    > +void osd_req_set_master_key(struct osd_request *, ...);/* NI */
    > +void osd_req_format(struct osd_request *, u64 tot_capacity);
    > +int osd_req_list_dev_partitions(struct osd_request *,
    > + osd_id initial_id, struct osd_obj_id_list *list, unsigned nelem);


    hm. It appears that someone made the decision to omit the name from
    the `struct osd_request *' parameter in the prototypes and to include
    the argument names for all other arguments.

    Not a bad idea, really.

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

  2. Re: [PATCHSET 00/18] open-osd: OSD Initiator library for Linux

    On Tue, 04 Nov 2008 18:09:31 +0200
    Boaz Harrosh wrote:

    >
    > Please consider for inclusion, an in-kernel OSD initiator
    > library. Its main users are planned to be various OSD based file
    > systems and the pNFS-Objects Layout Driver. (To be submitted soon)
    >
    > To try out and run the library please visit
    > http://open-osd.org and follow the instructions there.
    >
    > The submitted patchset is also available via git at:
    > git://git.open-osd.org/linux-open-osd.git osd
    > http://git.open-osd.org/gitweb.cgi?p...shortlog;h=osd
    >
    > Or a compact out-of-tree repository that includes sources
    > and some extras:
    > git://git.open-osd.org/open-osd.git master
    > http://git.open-osd.org/gitweb.cgi?p....git;a=summary
    >
    > ...
    >
    > We would like this to sit in -mm tree for a while to make sure it is compilable
    > on all platform.


    The best way to do that is to include your git tree in linux-next. If
    this code has a probably-will-be-merged-in-2.6.29 status then please
    prepare a branch for Stephen to include in the linux-next lineup.

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

  3. Re: [PATCH 03/18] libosd: OSDv1 Headers

    On Tue, 4 November 2008 11:10:37 -0800, Andrew Morton wrote:
    >
    > etc, etc, etc. Please review all that.


    One easy way to review 98% is to include the header in an otherwise
    empty foo.c and compile that. The remaining 2% are hidden behind
    different config options and architectures. Although your header may be
    fine on that front.

    Jörn

    --
    He who knows others is wise.
    He who knows himself is enlightened.
    -- Lao Tsu
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 03/18] libosd: OSDv1 Headers

    On Tue, 4 November 2008 20:42:40 +0100, Jörn Engel wrote:
    > On Tue, 4 November 2008 11:10:37 -0800, Andrew Morton wrote:
    > >
    > > etc, etc, etc. Please review all that.

    >
    > One easy way to review 98% is to include the header in an otherwise
    > empty foo.c and compile that. The remaining 2% are hidden behind
    > different config options and architectures. Although your header may be
    > fine on that front.


    Btw, running such a script on include/linux detects a lot of trouble
    even with i386 allnoconfig. Most of which looks it could be 'fixed' by
    a drunken monkey inserting #includes. But some of those #includes may
    be fs.h and pull the kitchensink, adding tons of dependencies and some
    compile time.

    Unsure.

    Jörn

    --
    Rules of Optimization:
    Rule 1: Don't do it.
    Rule 2 (for experts only): Don't do it yet.
    -- M.A. Jackson
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH 03/18] libosd: OSDv1 Headers

    Andrew Morton wrote:
    > On Tue, 4 Nov 2008 18:44:06 +0200
    > Boaz Harrosh wrote:
    >
    >> Headers only patch.
    >>
    >> osd_protocol.h
    >> Contains a C-fied definition of the T10 OSD standard
    >> osd_types.h
    >> Contains CPU order common used types
    >> osd_initiator.h
    >> API definition of the osd_initiator library
    >> osd_sec.h
    >> Contains High level API for the security manager.
    >>
    >> [Note that checkpatch spews errors on things that are valid in this context
    >> and will not be fixed]
    >>
    >> --- /dev/null
    >> +++ b/include/scsi/osd_initiator.h

    >
    > This header uses quite a few things without including the header files
    > which define them. That's a bit risky - it causes compile breakage
    > across architectures, across config changes and across time.
    >


    It's OK. I'm very pedantic about these things.

    The first header included at osd_initiator.c is this header precisely
    for the check Jörn has suggested. What happens is that all the types
    in osd_initiator.h are actually derived from osd_protocol.h below.
    osd_protocol.h does include exactly what it needs. (Also checked)

    The only new definitions used by this header are from linux/blkdev.h
    hence included here.

    (I will re-check that osd_protocol.h is self-sustained)

    >> @@ -0,0 +1,332 @@
    >> +/*
    >> + * osd_initiator.h - OSD initiator API definition
    >> + *
    >> + * Copyright (C) 2008 Panasas Inc. All rights reserved.
    >> + *
    >> + * Authors:
    >> + * Boaz Harrosh
    >> + * Benny Halevy
    >> + *
    >> + * This program is free software; you can redistribute it and/or modify
    >> + * it under the terms of the GNU General Public License version 2
    >> + *
    >> + */
    >> +#ifndef __OSD_INITIATOR_H__
    >> +#define __OSD_INITIATOR_H__
    >> +
    >> +#include
    >> +
    >> +#include "osd_protocol.h"
    >> +#include "osd_types.h"
    >> +
    >> +/* Note: "NI" in comments below means "Not Implemented yet" */
    >> +
    >> +/*
    >> + * Object-based Storage Device.
    >> + * This object represents an OSD device.
    >> + * It is not a full linux device in any way. It is only
    >> + * a place to hang resources associated with a Linux
    >> + * request Q and some default properties.
    >> + */
    >> +struct osd_dev {
    >> + struct scsi_device *scsi_dev;

    >
    > scsi_device
    >
    >> + unsigned def_timeout;
    >> +};
    >> +
    >> +void osd_dev_init(struct osd_dev *, struct scsi_device *scsi_dev);
    >> +void osd_dev_fini(struct osd_dev *);
    >> +
    >> +struct osd_request;
    >> +typedef void (osd_req_done_fn)(struct osd_request *, void *);
    >> +
    >> +struct osd_request {
    >> + struct osd_cdb cdb;
    >> + struct osd_data_out_integrity_info out_data_integ;
    >> + struct osd_data_in_integrity_info in_data_integ;
    >> +
    >> + struct osd_dev *osd_dev;
    >> + struct request *request;
    >> +
    >> + struct _osd_req_data_segment {
    >> + void *buff;
    >> + unsigned alloc_size; /* 0 here means not allocated by us */
    >> + unsigned total_bytes;
    >> + } set_attr, enc_get_attr, get_attr;
    >> +
    >> + struct _osd_io_info {
    >> + struct bio *bio;
    >> + u64 total_bytes;

    >
    > u64(!)
    >


    Do you mean that I need to use __u64? or what do you mean?

    I will change it. but just for curiosity, I have seen this mentioned
    once before, but never understood. What's wrong with the uXX types?
    I include linux/types.h and I get them as well as the __uXX set.
    Why are we not suppose to use them? And if we are not, then why do
    they exist? And also why the u8 is used everywhere but the u{16-64}
    are not?

    (Please for give me for attacking so strongly I just personally like
    them, style wise. I'm just sad to see them go )

    >> + struct request *req;
    >> + struct _osd_req_data_segment *last_seg;
    >> + u8 *pad_buff;
    >> + } out, in;
    >> +
    >> + gfp_t alloc_flags;

    >
    > gfp_t
    >


    I prefer my name. I've seen the gfp_t use in Kernel, but I needed
    that name while thinking the code. But now it's OK I'll change it.

    >> + unsigned timeout;
    >> + unsigned retries;
    >> + u8 sense[OSD_MAX_SENSE_LEN];
    >> + enum osd_attributes_mode attributes_mode;
    >> +
    >> + osd_req_done_fn *async_done;
    >> + void *async_private;
    >> + int async_error;
    >> +};

    >
    > etc, etc, etc. Please review all that.
    >


    You mean the uXX => __uXX? I'll change all that.

    >> +struct osd_request *osd_start_request(struct osd_dev *, gfp_t gfp);
    >> +int osd_finalize_request(struct osd_request *or,
    >> + u8 options, const void *cap, const u8 *cap_key);
    >> +void osd_req_set_master_seed_xchg(struct osd_request *, ...);/* NI */
    >> +void osd_req_set_master_key(struct osd_request *, ...);/* NI */
    >> +void osd_req_format(struct osd_request *, u64 tot_capacity);
    >> +int osd_req_list_dev_partitions(struct osd_request *,
    >> + osd_id initial_id, struct osd_obj_id_list *list, unsigned nelem);

    >
    > hm. It appears that someone made the decision to omit the name from
    > the `struct osd_request *' parameter in the prototypes and to include
    > the argument names for all other arguments.
    >
    > Not a bad idea, really.
    >


    It's a programing style thing. The "this" parameter name is dropped
    for been obvious and redundant. I like to keep the Header files
    most readable and self-documenting. I know that in C, the style is
    to look at .c files for answers, but I borrowed the C++ way of
    making it as easy as possible on the user and summarizing all exported
    types and services at the header file. Not just for the sake of the compiler
    but also for the reader.

    Note that I put the kernel-doc comments in the header and not in the
    source file.

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

  6. Re: [PATCH 03/18] libosd: OSDv1 Headers

    Jörn Engel wrote:
    > On Tue, 4 November 2008 11:10:37 -0800, Andrew Morton wrote:
    >> etc, etc, etc. Please review all that.

    >
    > One easy way to review 98% is to include the header in an otherwise
    > empty foo.c and compile that. The remaining 2% are hidden behind
    > different config options and architectures. Although your header may be
    > fine on that front.
    >
    > Jörn
    >


    I have done just that. It's an old habit of mine. The very first
    #include in osd_initiator.c is osd_initiator.h precisely for that.
    So it's not just a one time test it's always tested.

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

  7. Re: [PATCH 03/18] libosd: OSDv1 Headers

    On Wed, 2008-11-05 at 14:54 +0200, Boaz Harrosh wrote:
    > >> + struct _osd_io_info {
    > >> + struct bio *bio;
    > >> + u64 total_bytes;

    > >
    > > u64(!)
    > >

    >
    > Do you mean that I need to use __u64? or what do you mean?


    He means you've used u64 in a header without actually including any file
    that defines the typedef. Linux header files aren't supposed to depend
    on include order. They're supposed to stand alone. The point is that
    if I include just #include osd_initiator.h into an empty kernel file
    it's not supposed to spit undefined errors.

    Right at the moment the u64 probably works because blkdev.h #includes
    the file which defines it, but you're not supposed to rely on that.

    James


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

  8. Re: [PATCH 04/18] libosd: OSDv1 preliminary implementation

    Sam Ravnborg wrote:

    > On Tue, Nov 04, 2008 at 06:44:29PM +0200, Boaz Harrosh wrote:
    >> Implementation of the most basic OSD functionality and
    >> infrastructure. Mainly Format, Create/Remove Partition,
    >> Create/Remove Object, and read/write.
    >>
    >> - Add Makefile and Kbuild to compile libosd.ko
    >> - osd_initiator.c Implementation file for osd_initiator.h
    >> and osd_sec.h APIs
    >> - osd_debug.h - Some kprintf macro definitions

    >
    > A few comments below.
    >
    > Sam
    >


    Thanks Sam for looking

    >
    >> Signed-off-by: Boaz Harrosh
    >> Reviewed-by: Benny Halevy
    >> ---
    >> drivers/scsi/osd/Kbuild | 26 +++
    >> drivers/scsi/osd/Makefile | 37 +++
    >> drivers/scsi/osd/osd_debug.h | 27 +++
    >> drivers/scsi/osd/osd_initiator.c | 450 ++++++++++++++++++++++++++++++++++++++
    >> 4 files changed, 540 insertions(+), 0 deletions(-)
    >> create mode 100644 drivers/scsi/osd/Kbuild
    >> create mode 100755 drivers/scsi/osd/Makefile
    >> create mode 100644 drivers/scsi/osd/osd_debug.h
    >> create mode 100644 drivers/scsi/osd/osd_initiator.c
    >>
    >> diff --git a/drivers/scsi/osd/Kbuild b/drivers/scsi/osd/Kbuild
    >> new file mode 100644
    >> index 0000000..b4678e0
    >> --- /dev/null
    >> +++ b/drivers/scsi/osd/Kbuild
    >> @@ -0,0 +1,26 @@
    >> +#
    >> +# Kbuild for the OSD modules
    >> +#
    >> +# Copyright (C) 2008 Panasas Inc. All rights reserved.
    >> +#
    >> +# Authors:
    >> +# Boaz Harrosh
    >> +# Benny Halevy
    >> +#
    >> +# This program is free software; you can redistribute it and/or modify
    >> +# it under the terms of the GNU General Public License version 2
    >> +#
    >> +
    >> +ifneq ($(OSD_INC),)
    >> +# we are built out-of-tree Kconfigure everything as on
    >> +
    >> +CONFIG_SCSI_OSD_INITIATOR=m
    >> +EXTRA_CFLAGS += -DCONFIG_SCSI_OSD_INITIATOR -DCONFIG_SCSI_OSD_INITIATOR_MODULE
    >> +
    >> +EXTRA_CFLAGS += -I$(OSD_INC)
    >> +# EXTRA_CFLAGS += -DCONFIG_SCSI_OSD_DEBUG
    >> +
    >> +endif
    >> +
    >> +libosd-objs := osd_initiator.o
    >> +obj-$(CONFIG_SCSI_OSD_INITIATOR) += libosd.o

    >
    > When you submit for inclusion please clean this up.
    > 1) use ccflags-y as replacement for EXTRA_CFLAGS
    > 2) use libosd-y as replacement for libosd-objs
    >


    This is a most valuable information thanks. I have copy
    pasted these stuff, while learning. I guess from the wrong
    example. Thanks it looks much more logical that way.

    >> +
    >> +#ifdef CONFIG_SCSI_OSD_INITIATOR_MODULE
    >> +MODULE_AUTHOR("Boaz Harrosh ");
    >> +MODULE_DESCRIPTION("open-osd initiator library libosd.ko");
    >> +MODULE_LICENSE("GPL");
    >> +#endif

    >
    > no ifdef around here.
    >


    Grate, thanks. Again the wrong copy-paste.

    >> +void osd_dev_init(struct osd_dev *osdd, struct scsi_device *scsi_dev)
    >> +{
    >> + memset(osdd, 0, sizeof(*osdd));
    >> + osdd->scsi_dev = scsi_dev;
    >> + osdd->def_timeout = BLK_DEFAULT_SG_TIMEOUT;
    >> + /* TODO: Allocate pools for osd_request attributes ... */
    >> +}
    >> +EXPORT_SYMBOL(osd_dev_init);

    > kernel-doc comments for all exported funtions / variables.
    >


    I have some kernel-doc comments of exported functions in the Header
    file. I have not yet finished all of them. (Laziness on my part).

    Are kernel-doc comments in headers a big NO-NO. I like it this way,
    so when I have to learn a new Library all the information
    I need to know is in the header. Also the header is a much better place
    when you do programing by shopping, that is you don't know what you need
    and you look for what's available.

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

  9. Re: [PATCH 03/18] libosd: OSDv1 Headers

    James Bottomley wrote:
    > On Wed, 2008-11-05 at 14:54 +0200, Boaz Harrosh wrote:
    >>>> + struct _osd_io_info {
    >>>> + struct bio *bio;
    >>>> + u64 total_bytes;
    >>> u64(!)
    >>>

    >> Do you mean that I need to use __u64? or what do you mean?

    >
    > He means you've used u64 in a header without actually including any file
    > that defines the typedef. Linux header files aren't supposed to depend
    > on include order. They're supposed to stand alone. The point is that
    > if I include just #include osd_initiator.h into an empty kernel file
    > it's not supposed to spit undefined errors.
    >
    > Right at the moment the u64 probably works because blkdev.h #includes
    > the file which defines it, but you're not supposed to rely on that.
    >
    > James
    >


    I have addressed that issue. The osd_initiator.h includes the
    osd_protocol.h file. osd_protocol.h does include all the types
    needed. All of the osd_initiator.h types are derived from
    osd_protocol.h, the only added definitions are from blkdev.h.

    The very first #include in osd_initiator.c is osd_initiator.h
    which preforms exactly the check you suggest.

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

  10. Re: [PATCH 04/18] libosd: OSDv1 preliminary implementation

    Andrew Morton wrote:
    > On Tue, 4 Nov 2008 18:44:29 +0200
    > Boaz Harrosh wrote:
    >
    >> Implementation of the most basic OSD functionality and
    >> infrastructure. Mainly Format, Create/Remove Partition,
    >> Create/Remove Object, and read/write.
    >>
    >>
    >> ...
    >>
    >> +struct osd_request *_osd_request_alloc(gfp_t gfp)
    >> +{
    >> + struct osd_request *or;
    >> +
    >> + /* TODO: Use mempool with one saved request */
    >> + or = kzalloc(sizeof(*or), gfp);
    >> + return or;
    >> +}
    >> +
    >> +void _osd_request_free(struct osd_request *or)
    >> +{
    >> + kfree(or);
    >> +}

    >
    > These two functions can/should be made static. Please generally check
    > for this.
    >


    Thanks, I usually do, but these are from the last iteration and were
    rushed in. Will fix.

    > Also it'd probably make sense to declare both these inline. The
    > compiler _shoudl_ get it right, but stranger things have happened...
    >


    I do not inline them, because one - I will use mem_pools very soon they
    are just place holders for now. two - I let the compiler
    do that, I made sure the only user is below the definition and I let
    the compiler decide. I like to leave these things controlled from outside,
    so when I compile a UML kernel and finally need to fire up a debugger,
    I can un-inline them very easily.

    (This is why I hate forward declarations. If they are not used
    it is a proof that inlineing of single callers will always happen.)

    >> ...
    >>
    >> +/*
    >> + * If osd_finalize_request() was called but the request was not executed through
    >> + * the block layer, then we must release BIOs.
    >> + */
    >> +static void _abort_unexecuted_bios(struct request *rq)
    >> +{
    >> + struct bio *bio;
    >> +
    >> + while ((bio = rq->bio) != NULL) {
    >> + rq->bio = bio->bi_next;
    >> + bio_endio(bio, 0);
    >> + }
    >> +}

    >
    > Boy, that's a scary function. bye-bye data.
    >


    Thank's for mentioning that. I use it at the very end just before
    the de-allocation of the block request. What happens today is: that
    if for some reason the driver failed to call blk_end_request,
    or in this case the driver was never called, the last blk_put_request()
    will leak BIOs. There are currently corner cases and bugs in the Kernel
    that cause exactly that.

    That loop above should be moved from here to inside blk_put_request().
    if some one needs to hold the BIOs longer then the life span of the request they
    should simply inc-ref them.

    Note that here it is totally safe since It's only called just before
    blk_put_request().

    This code is actually a bug fix, for the error cases when a request is allocated
    but is never executed do to other error returns.

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

  11. Re: [PATCHSET 00/18] open-osd: OSD Initiator library for Linux

    Andrew Morton wrote:
    > On Tue, 04 Nov 2008 18:09:31 +0200
    > Boaz Harrosh wrote:
    >
    >> Please consider for inclusion, an in-kernel OSD initiator
    >> library. Its main users are planned to be various OSD based file
    >> systems and the pNFS-Objects Layout Driver. (To be submitted soon)
    >>
    >> To try out and run the library please visit
    >> http://open-osd.org and follow the instructions there.
    >>
    >> The submitted patchset is also available via git at:
    >> git://git.open-osd.org/linux-open-osd.git osd
    >> http://git.open-osd.org/gitweb.cgi?p...shortlog;h=osd
    >>
    >> Or a compact out-of-tree repository that includes sources
    >> and some extras:
    >> git://git.open-osd.org/open-osd.git master
    >> http://git.open-osd.org/gitweb.cgi?p....git;a=summary
    >>
    >> ...
    >>
    >> We would like this to sit in -mm tree for a while to make sure it is compilable
    >> on all platform.

    >
    > The best way to do that is to include your git tree in linux-next. If
    > this code has a probably-will-be-merged-in-2.6.29 status then please
    > prepare a branch for Stephen to include in the linux-next lineup.
    >


    Thanks Andrew
    I was thinking this code would eventually go into Linux-next threw
    James tree. But thought if there was a way for some exposure, so to
    give James some confidence in the code before he commits to it. But
    this way it could be perfect, thanks.

    Stephen Hi
    I have just that branch ready it is:
    git://git.open-osd.org/linux-open-osd.git osd
    Note the "osd" branch. Do I need to call it something special like
    "linux-next"? (Give me 24 hours to fix all current comments)

    Also one more question. This tree is based on latest scsi-misc-2.6.git,
    Because it is destined to go threw James tree at the end. Do I need
    to prepare a special tree based on Linus-latest?

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

  12. [Patch] Always include <linux/types.h>

    Hardly any file in the kernel can be compiled without including
    , directly or indirectly. And I'd wager a beer that
    noone can find a non-trivial example. I couldn't.

    So instead of sprinkling even more #include everywhere -
    140 headers in include/linux/ would need that to compile standalone -
    let us just pass it automatically.

    The existing 4000 odd includes for types.h, plus some 300 each for
    compiler.h and stddef.h, which get pulled through types.h, can get
    removed at leasure.

    diff --git a/Makefile b/Makefile
    index 6192922..8a3fb66 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -326,7 +326,8 @@ AFLAGS_KERNEL =
    # Needed to be compatible with the O= option
    LINUXINCLUDE := -Iinclude \
    $(if $(KBUILD_SRC),-Iinclude2 -I$(srctree)/include) \
    - -include include/linux/autoconf.h
    + -include include/linux/autoconf.h \
    + -include include/linux/types.h

    KBUILD_CPPFLAGS := -D__KERNEL__ $(LINUXINCLUDE)

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

  13. Re: [Patch] Always include <linux/types.h>

    On Wed, Nov 05, 2008 at 05:39:41PM +0100, Jrn Engel wrote:
    > Hardly any file in the kernel can be compiled without including
    > , directly or indirectly. And I'd wager a beer that
    > noone can find a non-trivial example. I couldn't.
    >
    > So instead of sprinkling even more #include everywhere -
    > 140 headers in include/linux/ would need that to compile standalone -
    > let us just pass it automatically.
    >
    > The existing 4000 odd includes for types.h, plus some 300 each for
    > compiler.h and stddef.h, which get pulled through types.h, can get
    > removed at leasure.
    >
    > --- a/Makefile
    > +++ b/Makefile
    > @@ -326,7 +326,8 @@ AFLAGS_KERNEL =
    > # Needed to be compatible with the O= option
    > LINUXINCLUDE := -Iinclude \
    > $(if $(KBUILD_SRC),-Iinclude2 -I$(srctree)/include) \
    > - -include include/linux/autoconf.h
    > + -include include/linux/autoconf.h \
    > + -include include/linux/types.h


    This is only going to slow down compilation and types.h is not causing much
    compilation problems, in fact, I can't recall a compilation problem due to
    types.h. Contary to config.h situation which was a pain.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [Patch] Always include <linux/types.h>

    Jörn Engel wrote:
    > Hardly any file in the kernel can be compiled without including
    > , directly or indirectly. And I'd wager a beer that
    > noone can find a non-trivial example. I couldn't.
    >
    > So instead of sprinkling even more #include everywhere -
    > 140 headers in include/linux/ would need that to compile standalone -
    > let us just pass it automatically.
    >
    > The existing 4000 odd includes for types.h, plus some 300 each for
    > compiler.h and stddef.h, which get pulled through types.h, can get
    > removed at leasure.
    >
    > diff --git a/Makefile b/Makefile
    > index 6192922..8a3fb66 100644
    > --- a/Makefile
    > +++ b/Makefile
    > @@ -326,7 +326,8 @@ AFLAGS_KERNEL =
    > # Needed to be compatible with the O= option
    > LINUXINCLUDE := -Iinclude \
    > $(if $(KBUILD_SRC),-Iinclude2 -I$(srctree)/include) \
    > - -include include/linux/autoconf.h
    > + -include include/linux/autoconf.h \
    > + -include include/linux/types.h
    >
    > KBUILD_CPPFLAGS := -D__KERNEL__ $(LINUXINCLUDE)
    >
    > --


    I think that if:
    [A]

    /* no includes pure level A header */


    [B]

    #include "header1.h"
    /* Level B header depends on level A */


    [C]

    #include "header2.h"
    use some types of header1.h
    /* Level C header depends on level B */


    [D]

    #include "header3.h"
    use types from A, B, C


    Then that's fine any other file that includes any one of A, B, or C will have
    no problem compiling and headers include order does not matter. Actually it is
    nice, since the reader of header3.h knows that it is derived/dependent work of
    header2.h.

    Now what happens in the future when at B #include "header1.h" is removed.
    At C header3.h stops compiling. So here I think it is the programmer's decision.
    If he thinks that usage of A types used at C are do to B and if B's implementation
    changes to use another set of types, then also C should change with it. Then leave
    it as above. If the programmer decides that there is independent use of A types
    in C unrelated to B, then he should also include A directly. If in doubt just don't
    include it.

    In any case I'm still not breaking the self-contained / order-independent headers
    rule.

    So please don't do that, Most of these places you found are the A-B-C case

    Just my $0.017
    Boaz

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

  15. Re: [Patch] Always include <linux/types.h>

    [ Threading should have been broken. Doh! ]

    On Wed, 5 November 2008 20:23:12 +0300, Alexey Dobriyan wrote:
    > On Wed, Nov 05, 2008 at 05:39:41PM +0100, Jörn Engel wrote:
    > > Hardly any file in the kernel can be compiled without including
    > > , directly or indirectly. And I'd wager a beer that
    > > noone can find a non-trivial example. I couldn't.

    >
    > This is only going to slow down compilation and types.h is not causing much
    > compilation problems, in fact, I can't recall a compilation problem due to
    > types.h. Contary to config.h situation which was a pain.


    My hope was actually to speed up compilation. If the average c file
    includes 10 headers, on types.h will get included by most of them,
    possibly multiple times. Each run after the first still has to parse
    the whole file, just to drop everything between #ifndef _LINUX_TYPES_H
    and #endif.

    By passing types.h on the command line we can drop it from all headers
    and only have to parse it once. Just the intermediate step of parsing
    types.h 11 times instead of 10 will slow things down. By about .4% on
    my not very beefy notebook.

    Before:
    real 4m33.241s
    user 3m58.524s
    sys 0m18.539s

    After:
    real 4m29.707s
    user 3m59.674s
    sys 0m18.182s

    And while testing this I noticed that a_flags shouldn't include the
    file. Updated patch below.

    Jörn

    --
    to show off how geeky they were.
    -- Rob Enderle

    diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
    index ea48b82..328f345 100644
    --- a/scripts/Makefile.lib
    +++ b/scripts/Makefile.lib
    @@ -121,7 +121,8 @@ endif

    c_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(KBUILD_CPPFLAGS) \
    $(__c_flags) $(modkern_cflags) \
    - -D"KBUILD_STR(s)=\#s" $(basename_flags) $(modname_flags)
    + -D"KBUILD_STR(s)=\#s" $(basename_flags) $(modname_flags) \
    + -include include/linux/types.h

    a_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(KBUILD_CPPFLAGS) \
    $(__a_flags) $(modkern_aflags)
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [Patch] Always include <linux/types.h>

    Jrn Engel writes:

    > My hope was actually to speed up compilation. If the average c file
    > includes 10 headers, on types.h will get included by most of them,
    > possibly multiple times. Each run after the first still has to parse
    > the whole file, just to drop everything between #ifndef _LINUX_TYPES_H
    > and #endif.


    Actually GCC is smart enought to note the bracketing and avoid even
    looking at it a second time.

    Andreas.

    --
    Andreas Schwab, SuSE Labs, schwab@suse.de
    SuSE Linux Products GmbH, Maxfeldstrae 5, 90409 Nrnberg, Germany
    PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
    "And now for something completely different."
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [Patch] Always include <linux/types.h>

    On Wed, 5 November 2008 20:16:28 +0100, Jörn Engel wrote:
    >
    > My hope was actually to speed up compilation. If the average c file
    > includes 10 headers, on types.h will get included by most of them,
    > possibly multiple times. Each run after the first still has to parse
    > the whole file, just to drop everything between #ifndef _LINUX_TYPES_H
    > and #endif.
    >
    > By passing types.h on the command line we can drop it from all headers
    > and only have to parse it once. Just the intermediate step of parsing
    > types.h 11 times instead of 10 will slow things down. By about .4% on
    > my not very beefy notebook.
    >
    > Before:
    > real 4m33.241s
    > user 3m58.524s
    > sys 0m18.539s
    >
    > After:
    > real 4m29.707s
    > user 3m59.674s
    > sys 0m18.182s


    And after removing all explicit #include from headers:
    real 4m31.946s
    user 3m59.521s
    sys 0m18.752s

    All this may be lost in the noise. The build machine wasn't completely
    idle and the variation in system time is rather random. So it neither
    helps nor hurts much. Nor does it solve any real problem.

    We might just as well drop it, I guess.

    Jörn

    --
    The story so far:
    In the beginning the Universe was created. This has made a lot
    of people very angry and been widely regarded as a bad move.
    -- Douglas Adams
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [Patch] Always include <linux/types.h>

    On Wed, Nov 05, 2008 at 08:16:29PM +0100, Jrn Engel wrote:
    > [ Threading should have been broken. Doh! ]
    >
    > On Wed, 5 November 2008 20:23:12 +0300, Alexey Dobriyan wrote:
    > > On Wed, Nov 05, 2008 at 05:39:41PM +0100, Jrn Engel wrote:
    > > > Hardly any file in the kernel can be compiled without including
    > > > , directly or indirectly. And I'd wager a beer that
    > > > noone can find a non-trivial example. I couldn't.

    > >
    > > This is only going to slow down compilation and types.h is not causing much
    > > compilation problems, in fact, I can't recall a compilation problem due to
    > > types.h. Contary to config.h situation which was a pain.

    >
    > My hope was actually to speed up compilation. If the average c file
    > includes 10 headers, on types.h will get included by most of them,
    > possibly multiple times. Each run after the first still has to parse
    > the whole file, just to drop everything between #ifndef _LINUX_TYPES_H
    > and #endif.
    >
    > By passing types.h on the command line we can drop it from all headers
    > and only have to parse it once. Just the intermediate step of parsing
    > types.h 11 times instead of 10 will slow things down. By about .4% on
    > my not very beefy notebook.
    >
    > Before:
    > real 4m33.241s
    > user 3m58.524s
    > sys 0m18.539s
    >
    > After:
    > real 4m29.707s
    > user 3m59.674s
    > sys 0m18.182s


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

  19. Re: [Patch] Always include <linux/types.h>

    On Wed, Nov 05, 2008 at 09:02:17PM +0100, Jrn Engel wrote:
    > On Wed, 5 November 2008 20:16:28 +0100, Jrn Engel wrote:
    > >
    > > My hope was actually to speed up compilation. If the average c file
    > > includes 10 headers, on types.h will get included by most of them,
    > > possibly multiple times. Each run after the first still has to parse
    > > the whole file, just to drop everything between #ifndef _LINUX_TYPES_H
    > > and #endif.
    > >
    > > By passing types.h on the command line we can drop it from all headers
    > > and only have to parse it once. Just the intermediate step of parsing
    > > types.h 11 times instead of 10 will slow things down. By about .4% on
    > > my not very beefy notebook.
    > >
    > > Before:
    > > real 4m33.241s
    > > user 3m58.524s
    > > sys 0m18.539s
    > >
    > > After:
    > > real 4m29.707s
    > > user 3m59.674s
    > > sys 0m18.182s

    >
    > And after removing all explicit #include from headers:
    > real 4m31.946s
    > user 3m59.521s
    > sys 0m18.752s
    >
    > All this may be lost in the noise. The build machine wasn't completely
    > idle and the variation in system time is rather random. So it neither
    > helps nor hurts much. Nor does it solve any real problem.
    >
    > We might just as well drop it, I guess.


    If you are working on improving compile times, it's better concentrate
    on removing unneeded includes. If just removing "extern"s from prototypes
    can reliably save several seconds, reducing headers can do wonders.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  20. Re: [Patch] Always include <linux/types.h>

    On Wed, 5 November 2008 23:32:12 +0300, Alexey Dobriyan wrote:
    >
    > If you are working on improving compile times, it's better concentrate
    > on removing unneeded includes. If just removing "extern"s from prototypes
    > can reliably save several seconds, reducing headers can do wonders.


    My goal was more to assume we want all headers to compile standalone and
    see where that would lead. Result for include/linux/ was some 500 added
    lines, 170 of which were to add types.h or compiler.h. Another 50-100
    further includes were fairly well-spread across the spectrum. The
    remainder was declarations like
    struct super_block;

    Overall we have three evils to choose from. Headers with unresolved
    dependencies lead to random compile breakage after removing a header
    from some .c file. Even if the change was tested, it can still break
    for some config/architecture combination month down the line.

    Sprinkling more includes throughout the headers increase compile time.
    And the common practice of declaring a structure instead of including
    the header is a pita when working with ctags. The last problem is
    particularly annoying since I have no idea what problem this warning is
    supposed to solve:
    In file included from include/linux/coda_cache.c:1:
    include/linux/coda_cache.h:18: warning: ‘struct super_block’ declared inside parameter list
    include/linux/coda_cache.h:18: warning: its scope is only this definition or declaration, which is probably not what you want

    Should we just teach gcc to shut up about that one?

    Jörn

    --
    Anything that can go wrong, will.
    -- Finagle's Law
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 2 of 3 FirstFirst 1 2 3 LastLast