Messages in this thread | | | Date | Sat, 31 Jan 2015 16:50:59 +0800 | Subject | Re: [PATCH v1 2/2] zram: remove init_lock in zram_make_request | From | Ganesh Mahendran <> |
| |
2015-01-30 16:08 GMT+08:00 Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>: > On (01/30/15 15:52), Ganesh Mahendran wrote: >> >> When I/O operation is running, that means the /dev/zram0 is >> >> mounted or swaped on. Then the device could not be reset by >> >> below code: >> >> >> >> /* Do not reset an active device! */ >> >> if (bdev->bd_holders) { >> >> ret = -EBUSY; >> >> goto out; >> >> } >> >> >> >> So the zram->init_lock in I/O path is to check whether the device >> >> has been initialized(echo xxx > /sys/block/zram/disk_size). >> >> >> >> Thanks for your explanation. >> >> > >> > for mounted device (w/fs), we see initial (well, it goes up and down >> >> What does "w/" mean? > > 'with fs' > >> > many times while we create device, but this is not interesting here) >> > ->bd_holders increment in: >> > vfs_kern_mount -> mount_bdev -> blkdev_get_by_path -> blkdev_get >> > >> > and it goes to zero in: >> > cleanup_mnt -> deactivate_super -> kill_block_super -> blkdev_put >> > >> > >> > after umount we still have init device. so, *theoretically*, we >> > can see something like >> > >> > CPU0 CPU1 >> > umount >> > reset_store >> > bdev->bd_holders == 0 mount >> > ... zram_make_request() >> > zram_reset_device() >> >> In this example, the data stored in zram will be corrupted. >> Since CPU0 will free meta while CPU1 is using. >> right? >> > > with out ->init_lock protection in this case we have 'free' vs. 'use' race.
Maybe I did not explain clearly. I send a patch about this issue:
https://patchwork.kernel.org/patch/5754041/
Thanks
> >> >> > >> > w/o zram->init_lock in both zram_reset_device() and zram_make_request() >> > one of CPUs will be a bit sad. >> what does "w/o" mean? > > 'with out' > > > -ss
| |