Re: [PATCH] struct char_device

[Anton Altaparmakov]

Hello,

At 15:18 22/05/01, Alexander Viro wrote:
[snip cool stuff]
>diff -urN S5-pre4/include/linux/fs.h S5-pre4-cdev/include/linux/fs.h
>--- S5-pre4/include/linux/fs.h  Sat May 19 22:46:36 2001
>+++ S5-pre4-cdev/include/linux/fs.h     Tue May 22 09:14:25 2001
>@@ -384,6 +384,14 @@
>         int                     gfp_mask;       /* how to allocate the 
> pages */
>  };
>
>+struct char_device {
>+       struct list_head        hash;
>+       atomic_t                count;
>+       dev_t                   dev;
>+       atomic_t                openers;
>+       struct semaphore        sem;

Why not name consistently with the struct block_device?
I.e.:
struct char_device {
         struct list_head        cd_hash;
         atomic_t                cd_count;
         dev_t                   cd_dev;
         atomic_t                cd_openers;
         struct semaphore        cd_sem;
};

>@@ -426,8 +434,10 @@
>         struct address_space    *i_mapping;
>         struct address_space    i_data;
>         struct dquot            *i_dquot[MAXQUOTAS];
>+       /* These three should probably be a union */
>         struct pipe_inode_info  *i_pipe;
>         struct block_device     *i_bdev;
>+       struct char_device      *i_cdev;

You could then use an (perhaps even unnamed if we require a high enough gcc 
version) union in the inode structure so you don't waste space having a 
pointer to both c_dev and b_dev, such as:

         union {
                 struct block_device     *i_bdev;
                 struct char_device      *i_cdev;
         };

It isn't possible that both i_bdev and i_cdev are used at the same time, is 
it? If it was my suggestion is obviously bogus...

Any reasons why my proposal would be a bad idea?

Just IMVHO.

Best regards,

Anton