netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: netdev@vger.kernel.org, Jiri Pirko <jiri@resnulli.us>,
	Jiri Pirko <jiri@mellanox.com>
Subject: Re: [PATCH 06/10] devlink: convert snapshot id getter to return an error
Date: Wed, 25 Mar 2020 11:04:25 -0700	[thread overview]
Message-ID: <20200325110425.6fdf6cb3@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20200324223445.2077900-7-jacob.e.keller@intel.com>

On Tue, 24 Mar 2020 15:34:41 -0700 Jacob Keller wrote:
> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
> index f7621ccb7b88..f9420b77e5fd 100644
> --- a/drivers/net/netdevsim/dev.c
> +++ b/drivers/net/netdevsim/dev.c
> @@ -45,8 +45,7 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file,
>  {
>  	struct nsim_dev *nsim_dev = file->private_data;
>  	void *dummy_data;
> -	int err;
> -	u32 id;
> +	int err, id;
>  
>  	dummy_data = kmalloc(NSIM_DEV_DUMMY_REGION_SIZE, GFP_KERNEL);
>  	if (!dummy_data)
> @@ -55,6 +54,10 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file,
>  	get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE);
>  
>  	id = devlink_region_snapshot_id_get(priv_to_devlink(nsim_dev));
> +	if (id < 0) {
> +		pr_err("Failed to get snapshot id\n");
> +		return id;
> +	}
>  	err = devlink_region_snapshot_create(nsim_dev->dummy_region,
>  					     dummy_data, id);
>  	if (err) {

Hmm... next patch introduces some ref counting on the ID AFAICT,
should there be some form of snapshot_id_put(), once the driver is 
done creating the regions it wants?

First what if driver wants to create two snapshots with the same ID but
user space manages to delete the first one before second one is created.

Second what if create fails, won't the snapshot ID just stay in XA with
count of 0 forever?

  reply	other threads:[~2020-03-25 18:04 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24 22:34 [PATCH 00/10] implement DEVLINK_CMD_REGION_NEW Jacob Keller
2020-03-24 22:34 ` [PATCH 01/10] devlink: prepare to support region operations Jacob Keller
2020-03-24 22:34 ` [PATCH 02/10] devlink: convert snapshot destructor callback to region op Jacob Keller
2020-03-24 22:34 ` [PATCH 03/10] devlink: trivial: fix tab in function documentation Jacob Keller
2020-03-24 22:34 ` [PATCH 04/10] devlink: add function to take snapshot while locked Jacob Keller
2020-03-25 17:56   ` Jakub Kicinski
2020-03-24 22:34 ` [PATCH 05/10] devlink: extract snapshot id allocation to helper function Jacob Keller
2020-03-25 14:08   ` Jiri Pirko
2020-03-24 22:34 ` [PATCH 06/10] devlink: convert snapshot id getter to return an error Jacob Keller
2020-03-25 18:04   ` Jakub Kicinski [this message]
2020-03-25 19:13     ` Jiri Pirko
2020-03-26  1:33     ` Jacob Keller
2020-03-24 22:34 ` [PATCH] devlink: track snapshot id usage count using an xarray Jacob Keller
2020-03-25 16:08   ` Jiri Pirko
2020-03-26  1:16     ` Jacob Keller
2020-03-26  1:43     ` Jacob Keller
2020-03-24 22:34 ` [PATCH 08/10] devlink: implement DEVLINK_CMD_REGION_NEW Jacob Keller
2020-03-25 16:46   ` Jiri Pirko
2020-03-25 17:18     ` Jakub Kicinski
2020-03-25 17:20       ` Jiri Pirko
2020-03-25 17:46         ` Jakub Kicinski
2020-03-25 18:41           ` Jiri Pirko
2020-03-26  1:30     ` Jacob Keller
2020-03-24 22:34 ` [PATCH 09/10] netdevsim: support taking immediate snapshot via devlink Jacob Keller
2020-03-25 16:50   ` Jiri Pirko
2020-03-24 22:34 ` [PATCH 10/10] ice: add a devlink region for dumping NVM contents Jacob Keller
2020-03-25 17:18   ` Jiri Pirko
2020-03-25 13:49 ` [PATCH 00/10] implement DEVLINK_CMD_REGION_NEW Jiri Pirko
2020-03-25 13:50 ` Jiri Pirko

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=20200325110425.6fdf6cb3@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=jacob.e.keller@intel.com \
    --cc=jiri@mellanox.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).