Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 13 Apr 2022 14:47:17 +0800
From:   Chao Peng <chao.p.peng@linux.intel.com>
To:     Hillf Danton <hdanton@sina.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH v5 02/13] mm: Introduce memfile_notifier
Message-ID: <20220413064717.GB10041@chaop.bj.intel.com>
Reply-To: Chao Peng <chao.p.peng@linux.intel.com>
References: <20220310140911.50924-3-chao.p.peng@linux.intel.com>
 <20220412143654.6313-1-hdanton@sina.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220412143654.6313-1-hdanton@sina.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Tue, Apr 12, 2022 at 10:36:54PM +0800, Hillf Danton wrote:
> On Thu, 10 Mar 2022 22:09:00 +0800 Chao Peng wrote:
> > +
> > +void memfile_register_backing_store(struct memfile_backing_store *bs)
> > +{
> > +	BUG_ON(!bs || !bs->get_notifier_list);
> > +
> > +	list_add_tail(&bs->list, &backing_store_list);
> > +}
> > +
> > +void memfile_unregister_backing_store(struct memfile_backing_store *bs)
> > +{
> > +	list_del(&bs->list);
> > +}
> > +
> > +static int memfile_get_notifier_info(struct inode *inode,
> 
> Nit, s/get/lookup/

Thanks.

> 
> > +				     struct memfile_notifier_list **list,
> > +				     struct memfile_pfn_ops **ops)
> > +{
> > +	struct memfile_backing_store *bs, *iter;
> > +	struct memfile_notifier_list *tmp;
> > +
> > +	list_for_each_entry_safe(bs, iter, &backing_store_list, list) {
> 
> Wonder what serializes list walk with list del and add above.

Yes, this needs locking if we want to support backing stores as modules,
as Sean pointed out, at this time this is not quite meaningful so I will
remove unregister also the register should be done at kernel init time
so the walk here is actually on a readonly list.

Thanks,
Chao

> 
> > +		tmp = bs->get_notifier_list(inode);
> > +		if (tmp) {
> > +			*list = tmp;
> > +			if (ops)
> > +				*ops = &bs->pfn_ops;
> > +			return 0;
> > +		}
> > +	}
> > +	return -EOPNOTSUPP;
> > +}