On Tue, Feb 07, 2017 at 11:24:40AM +0800, zhouxianrong wrote: > > > On 2017/2/7 10:54, Minchan Kim wrote: > >On Tue, Feb 07, 2017 at 10:20:57AM +0800, zhouxianrong wrote: > > > >< snip > > > > >>>>3. the below should be modified. > >>>> > >>>>static inline bool zram_meta_get(struct zram *zram) > >>>>@@ -495,11 +553,17 @@ static void zram_meta_free(struct zram_meta *meta, u64 disksize) > >>>> > >>>> /* Free all pages that are still in this zram device */ > >>>> for (index = 0; index < num_pages; index++) { > >>>>- unsigned long handle = meta->table[index].handle; > >>>>+ unsigned long handle; > >>>>+ > >>>>+ bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value); > >>>>+ handle = meta->table[index].handle; > >>>> > >>>>- if (!handle) > >>>>+ if (!handle || zram_test_flag(meta, index, ZRAM_SAME)) { > >>>>+ bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value); > >>>> continue; > >>>>+ } > >>>> > >>>>+ bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value); > >>>> zs_free(meta->mem_pool, handle); > >>> > >>>Could you explain why we need this modification? > >>> > >>>> } > >>>> > >>>>@@ -511,7 +575,7 @@ static void zram_meta_free(struct zram_meta *meta, u64 disksize) > >>>>static struct zram_meta *zram_meta_alloc(char *pool_name, u64 disksize) > >>>>{ > >>>> size_t num_pages; > >>>>- struct zram_meta *meta = kmalloc(sizeof(*meta), GFP_KERNEL); > >>>>+ struct zram_meta *meta = kzalloc(sizeof(*meta), GFP_KERNEL); > >>> > >>>Ditto > >>> > >>>> > >>>> > >>> > >>>. > >>> > >> > >>because of union of handle and element, i think a non-zero element (other than handle) is prevented from freeing. > >>if zram_meta_get was modified, zram_meta_alloc did so. > > > >Right. Thanks but I don't see why we need the locking in there and modification of > >zram_meta_alloc. > > > >Isn't it enough with this? > > i am afraid someone do reset_store, so did lock. reset_store is protected by zram->claim and and init_done so I don't think so. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>