KVM Archive on lore.kernel.org
 help / color / Atom feed
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: kvm@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	mst@redhat.com, borntraeger@de.ibm.com,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [RFC PATCH 2/5] statsfs API: create, add and remove statsfs sources and values
Date: Mon, 27 Apr 2020 18:48:09 +0200
Message-ID: <116c9fff-02c1-ddb4-feab-d836c79b1373@redhat.com> (raw)
In-Reply-To: <20200427154727.GH29705@bombadil.infradead.org>



On 4/27/20 5:47 PM, Matthew Wilcox wrote:
> On Mon, Apr 27, 2020 at 04:18:13PM +0200, Emanuele Giuseppe Esposito wrote:
>> +static struct statsfs_value *find_value(struct statsfs_value_source *src,
>> +					struct statsfs_value *val)
>> +{
>> +	struct statsfs_value *entry;
>> +
>> +	for (entry = src->values; entry->name; entry++) {
>> +		if (entry == val) {
>> +			WARN_ON(strcmp(entry->name, val->name) != 0);
> 
> Umm.  'entry' and 'val' are pointers.  So if entry is equal to val,
> how could entry->name and val->name not be the same thing?

Good catch, I'll get rid of that check.
> 
> 
> +int statsfs_source_add_values(struct statsfs_source *source,
> +			      struct statsfs_value *stat, void *ptr)
> +{
> +	struct statsfs_value_source *entry, *val_src;
> +	unsigned long index;
> +	int err = -EEXIST;
> +
> +	val_src = create_value_source(ptr);
> +	val_src->values = stat;
> +
> +	xa_lock(&source->values);
> +	xa_for_each(&source->values, index, entry) {
> +		if (entry->base_addr == ptr && entry->values == stat)
> +			goto out;
> +	}
> +
> +	err = __xa_alloc(&source->values, &val_src->id, val_src, xa_limit_32b,
> +			GFP_KERNEL);
> +out:
> +	xa_unlock(&source->values);
> +	if (err)
> +		kfree(val_src);
> +	return err;
> +}
> 
> Using an XArray avoids the occasional latency problems you can see with
> rwsems, as well as being more cache-effective than a linked list.

I didn't know about XArrays, I'll give them a look. I will also fix the 
list initialization with INIT_LIST_HEAD.

Thank you for the above example, but I don't think that each source 
would have more than 2 or 3 value_sources, so using a linked list there 
should be fine.
However, this might be a good point for the subordinates list.

Regarding the locking, the rwsem is also used to protect the other 
lists and dentry of the source, so it wouldn't be removed anyways.

Thank you,

Emanuele


  reply index

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-27 14:18 [RFC PATCH 0/5] Statsfs: a new ram-based file sytem for Linux kernel statistics Emanuele Giuseppe Esposito
2020-04-27 14:18 ` [RFC PATCH 1/5] refcount, kref: add dec-and-test wrappers for rw_semaphores Emanuele Giuseppe Esposito
2020-04-27 14:18 ` [RFC PATCH 2/5] statsfs API: create, add and remove statsfs sources and values Emanuele Giuseppe Esposito
2020-04-27 15:47   ` Matthew Wilcox
2020-04-27 16:48     ` Emanuele Giuseppe Esposito [this message]
2020-04-29  9:49     ` [RFC PATCH 2/5] statsfs API: create, add and remove statsfs Emanuele Giuseppe Esposito
2020-04-27 21:53   ` [RFC PATCH 2/5] statsfs API: create, add and remove statsfs sources and values Andreas Dilger
2020-04-29 10:55     ` Emanuele Giuseppe Esposito
2020-04-28 17:47   ` Randy Dunlap
2020-04-29 10:34     ` Paolo Bonzini
2020-04-27 14:18 ` [RFC PATCH 3/5] kunit: tests for statsfs API Emanuele Giuseppe Esposito
2020-04-28 17:50   ` Randy Dunlap
2020-04-27 14:18 ` [RFC PATCH 4/5] statsfs fs: virtual fs to show stats to the end-user Emanuele Giuseppe Esposito
2020-04-27 14:18 ` [RFC PATCH 5/5] kvm_main: replace debugfs with statsfs Emanuele Giuseppe Esposito
2020-04-28 17:56   ` Randy Dunlap
2020-04-29 10:34     ` Emanuele Giuseppe Esposito
2020-04-29 10:35       ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=116c9fff-02c1-ddb4-feab-d836c79b1373@redhat.com \
    --to=eesposit@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git