From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753615AbcARHDS (ORCPT ); Mon, 18 Jan 2016 02:03:18 -0500 Received: from mail-pa0-f65.google.com ([209.85.220.65]:33342 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751209AbcARHDP (ORCPT ); Mon, 18 Jan 2016 02:03:15 -0500 Date: Mon, 18 Jan 2016 16:04:27 +0900 From: Sergey Senozhatsky To: Minchan Kim Cc: Vlastimil Babka , Sergey Senozhatsky , Junil Lee , ngupta@vflare.org, sergey.senozhatsky.work@gmail.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] zsmalloc: fix migrate_zspage-zs_free race condition Message-ID: <20160118070427.GC459@swordfish> References: <1452843551-4464-1-git-send-email-junil0814.lee@lge.com> <20160115143434.GA25332@blaptop.local> <56991514.9000609@suse.cz> <20160116040913.GA566@swordfish> <5699F4C9.1070902@suse.cz> <20160116080650.GB566@swordfish> <5699FC69.4010000@suse.cz> <20160118063246.GB7453@bbox> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160118063246.GB7453@bbox> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, oh, you replied in this thread. On (01/18/16 15:32), Minchan Kim wrote: > > free_obj = obj_malloc(d_page, class, handle); > > zs_object_copy(free_obj, used_obj, class); > > index++; > > + /* This also effectively unpins the handle */ > > record_obj(handle, free_obj); > > - unpin_tag(handle); > > obj_free(pool, class, used_obj); > > } > > > > But I'd still recommend WRITE_ONCE in record_obj(). And I'm not even sure it's > > Thanks for the reivew. Yeah, we need WRITE_ONCE in record_obj but > your version will not work. IMHO, WRITE_ONCE can prevent store-tearing > but it couldn't prevent reordering. IOW, we need some barrier as unlock > and clear_bit_unlock includes it. > So, we shouldn't omit unpin_tag there. but there is only one store operation after this patch. static void record_obj(unsigned long handle, unsigned long obj) { *(unsigned long *)handle = obj; } does the re-ordering problem exist? zs_free() will see the old pinned handle and spin, until record_obj() from migrate. -ss > > safe on all architectures to do a simple overwrite of a word against somebody > > else trying to lock a bit there? > > Hmm, I think it shouldn't be a problem. It's word-alinged, word-sized > store so it should be atomic. > > As other example, we have been used lock_page for a bit of page->flags > and used other bits in there with __set_bit(ie, __SetPageXXX). > I guess it's same situation with us just except we are spinning there. > But it is worth to dobule check so need to help lock guys.