On 05/17/2010 05:31 PM, Minchan Kim wrote: > On Mon, May 17, 2010 at 2:32 PM, Nitin Gupta <ngupta@xxxxxxxxxx> wrote: >> This callback is required when RAM based devices are used as swap disks. >> One such device is ramzswap which is used as compressed in-memory swap >> disk. For such devices, we need a callback as soon as a swap slot is no >> longer used to allow freeing memory allocated for this slot. Without this >> callback, stale data can quickly accumulate in memory defeating the whole >> purpose of such devices. >> >> Signed-off-by: Nitin Gupta <ngupta@xxxxxxxxxx> > Reviewed-by: Minchan Kim <minchan.kim@xxxxxxxxx> > > Looks good to me about code. so I added my review sign. > But I have some comments. > > last time I said, I don't like there is a swap specific function in > block_device_operations. > It doesn't need many memory but it's not good about design sine > block_device_operations have common functions about block device. > > But I don't have any good idea now where I put swap specific function. > And Linus already acked this idea. Hmm. > > If there isn't any objection, I don't insist on my thought. > > Nitpick : > AFAIR, Nitin introduced SWP_BLKDEV since he think access of long > pointers isn't good. ex) > S_ISBLK(swap_info_struct->swap_file->f_mapping->host->i_mode) > > But now, we have to access p->bdev->bd_disk->fops->swap_slot_free_notify. > Isn't it all right? > I'm also not sure about this point but accessing yet another very long pointer chain just to check if its a block device seems weird. Anyways, its trivial to remove this swap flag if its later decided that its not really needed. Thanks, Nitin _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
