lkml.org 
[lkml]   [2010]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 2/2] Add BLKGETQUEUEINFO for reading block queue attributes
    Date
    On Thursday 09 December 2010 16:25:37 Lukas Czerner wrote:
    > +enum blkq_info_type {
    > + BLKQ_REQUESTS = 0,
    > + BLKQ_RA,
    > + BLKQ_MAX_HW_SECTORS,
    > + BLKQ_MAX_SECTORS,
    > + BLKQ_MAX_SEGMENTS,
    > + BLKQ_MAX_INTEGRITY_SEGMENTS,
    > + BLKQ_MAX_SEGMENT_SIZE,
    > + BLKQ_IOSCHED,
    > + BLKQ_HW_SECTOR_SIZE,
    > + BLKQ_LOGICAL_BLOCK_SIZE,
    > + BLKQ_PHYSICAL_BLOCK_SIZE,
    > + BLKQ_IO_MIN,
    > + BLKQ_IO_OPT,
    > + BLKQ_DISCARD_GRANULARITY,
    > + BLKQ_DISCARD_MAX,
    > + BLKQ_DISCARD_ZEROES_DATA,
    > + BLKQ_NONROT,
    > + BLKQ_NOMERGES,
    > + BLKQ_RQ_AFFINITY,
    > + BLKQ_IOSTATS,
    > + BLKQ_RANDOM,
    > + BLKQ_END, /* Last item = quantity of items */
    > +};
    > +
    > +struct blk_queue_info {
    > + enum blkq_info_type type;
    > + unsigned long data;
    > +};

    You are adding another indirection to an indirect system call
    here. Besides the problems that Greg mentioned, this is
    also really ugly. If it turns out that we want an ioctl interface
    for this after all, better make it either one command per value,
    or one data structure that contains all the values (plus some
    reserved fields for future extensions).

    Furthermore, you should use neither enum nor long data types
    in a data structure that is used as an ABI. Use either __u32
    or __u64 here, and make sure you have no padding in the middle
    or at the end if you mix the two.

    > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
    > index 30b8815..71bd06c 100644
    > --- a/include/linux/sysfs.h
    > +++ b/include/linux/sysfs.h
    > @@ -112,6 +112,7 @@ struct bin_attribute {
    > struct sysfs_ops {
    > ssize_t (*show)(struct kobject *, struct attribute *,char *);
    > ssize_t (*store)(struct kobject *,struct attribute *,const char *, size_t);
    > + ssize_t (*get)(struct kobject *, struct attribute *, void *);
    > };
    >

    This looks like it can significantly add to the .data size of the kernel.

    Arnd


    \
     
     \ /
      Last update: 2010-12-10 19:43    [W:0.021 / U:124.080 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site