On (04/23/15 12:06), Minchan Kim wrote:
> > +Example:
> > + cat /sys/class/zram-control/zram_add
>
> Why do we put zram-contol there rather than /sys/block/zram
that's what clsss_register() does.
[..]
> > @@ -1168,8 +1172,15 @@ static int zram_add(int device_id)
>
> Why do zram_add need device_id?
> We decided to remove option user pass device_id.
will cleanup. it was simpler at that time to support both devices
created by sysfs request and devices pre-crated for num_devices
module param.
> > +static struct zram *zram_lookup(int dev_id)
> > +{
> > + struct zram *zram;
> > +
> > + zram = idr_find(&zram_index_idr, dev_id);
> > + if (zram)
> > + return zram;
> > + return ERR_PTR(-ENODEV);
>
> Just return NULL which is more simple and readable.
>
ok.
[..]
> > + /*
> > + * First, make ->disksize device attr RO, closing
> > + * zram_remove() vs disksize_store() race window
>
> Why don't you use zram->init_lock to protect the race?
zram_reset_device() takes this lock internally. but, it
unlocks the device upon the return from zram_reset_device():
lock idr_lock
zram_reset_device()
lock bd_mutex
__zam_reset_device()
lock init_lock
reset
unlock init_lock ---\
unlock bd_mutex |
|<----- disksize_store() race window
zram_remove() ---/
unlock idr_lock
until we call zram_remove() (which does sysfs_remove_group()) device has
sysfs attrs and, thus, disksize_store() can arrive in the middle. the
simplest things I came up with was that RO bit on sysfs disksize attrs.
I can factor out another set of __foo function to handle it differently,
not sure if this worth it.
I'll revisit it.
-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
