All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Lobakin <aleksander.lobakin@intel.com>
To: Alan Brady <alan.brady@intel.com>
Cc: netdev@vger.kernel.org, willemdebruijn.kernel@gmail.com,
	intel-wired-lan@lists.osuosl.org, igor.bagnucki@intel.com,
	przemyslaw.kitszel@intel.com
Subject: Re: [Intel-wired-lan] [PATCH v2 0/7 iwl-next] idpf: refactor virtchnl messages
Date: Mon, 29 Jan 2024 14:23:35 +0100	[thread overview]
Message-ID: <41774365-e247-4e32-9e96-d256acfcc129@intel.com> (raw)
In-Reply-To: <20240126054747.960172-1-alan.brady@intel.com>

From: Alan Brady <alan.brady@intel.com>
Date: Thu, 25 Jan 2024 21:47:40 -0800

> The motivation for this series has two primary goals. We want to enable
> support of multiple simultaneous messages and make the channel more
> robust. The way it works right now, the driver can only send and receive
> a single message at a time and if something goes really wrong, it can
> lead to data corruption and strange bugs.

[...]

There are a fistful of functions in this series and IDPF's virtchnl code
in general that allocate a memory chunk via kzalloc() family and then
free it at the end of the function, i.e. the lifetime of those buffers
are the lifetime of the function.
Since recently, we have auto-variables in the kernel, so that the pieces
I described could be converted to:

	struct x *ptr __free(kfree) = NULL;

	ptr = kzalloc(sizeof(*x), GPF_KERNEL);

	// some code

	return 0; // kfree() is not needed anymore

err:
	return err; // here as well

That would allow to simplify the code and reduce its size.
I'd like you to convert such functions to use auto-variables.

Thanks,
Olek

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Lobakin <aleksander.lobakin@intel.com>
To: Alan Brady <alan.brady@intel.com>
Cc: <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
	<willemdebruijn.kernel@gmail.com>, <igor.bagnucki@intel.com>,
	<przemyslaw.kitszel@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH v2 0/7 iwl-next] idpf: refactor virtchnl messages
Date: Mon, 29 Jan 2024 14:23:35 +0100	[thread overview]
Message-ID: <41774365-e247-4e32-9e96-d256acfcc129@intel.com> (raw)
In-Reply-To: <20240126054747.960172-1-alan.brady@intel.com>

From: Alan Brady <alan.brady@intel.com>
Date: Thu, 25 Jan 2024 21:47:40 -0800

> The motivation for this series has two primary goals. We want to enable
> support of multiple simultaneous messages and make the channel more
> robust. The way it works right now, the driver can only send and receive
> a single message at a time and if something goes really wrong, it can
> lead to data corruption and strange bugs.

[...]

There are a fistful of functions in this series and IDPF's virtchnl code
in general that allocate a memory chunk via kzalloc() family and then
free it at the end of the function, i.e. the lifetime of those buffers
are the lifetime of the function.
Since recently, we have auto-variables in the kernel, so that the pieces
I described could be converted to:

	struct x *ptr __free(kfree) = NULL;

	ptr = kzalloc(sizeof(*x), GPF_KERNEL);

	// some code

	return 0; // kfree() is not needed anymore

err:
	return err; // here as well

That would allow to simplify the code and reduce its size.
I'd like you to convert such functions to use auto-variables.

Thanks,
Olek

  parent reply	other threads:[~2024-01-29 13:24 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-26  5:47 [Intel-wired-lan] [PATCH v2 0/7 iwl-next] idpf: refactor virtchnl messages Alan Brady
2024-01-26  5:47 ` Alan Brady
2024-01-26  5:47 ` [Intel-wired-lan] [PATCH v2 1/7 iwl-next] idpf: implement virtchnl transaction manager Alan Brady
2024-01-26  5:47   ` Alan Brady
2024-01-26  5:47 ` [Intel-wired-lan] [PATCH v2 2/7 iwl-next] idpf: refactor vport virtchnl messages Alan Brady
2024-01-26  5:47   ` Alan Brady
2024-01-26  5:47 ` [Intel-wired-lan] [PATCH v2 3/7 iwl-next] idpf: refactor queue related " Alan Brady
2024-01-26  5:47   ` Alan Brady
2024-01-26  5:47 ` [Intel-wired-lan] [PATCH v2 4/7 iwl-next] idpf: refactor remaining " Alan Brady
2024-01-26  5:47   ` Alan Brady
2024-01-26  5:47 ` [Intel-wired-lan] [PATCH v2 5/7 iwl-next] idpf: add async_handler for MAC filter messages Alan Brady
2024-01-26  5:47   ` Alan Brady
2024-01-26  5:47 ` [Intel-wired-lan] [PATCH v2 6/7 iwl-next] idpf: refactor idpf_recv_mb_msg Alan Brady
2024-01-26  5:47   ` Alan Brady
2024-01-26  5:47 ` [Intel-wired-lan] [PATCH v2 7/7 iwl-next] idpf: cleanup virtchnl cruft Alan Brady
2024-01-26  5:47   ` Alan Brady
2024-01-29 13:23 ` Alexander Lobakin [this message]
2024-01-29 13:23   ` [Intel-wired-lan] [PATCH v2 0/7 iwl-next] idpf: refactor virtchnl messages Alexander Lobakin
2024-01-29 15:12   ` Brady, Alan
2024-01-29 15:12     ` Brady, Alan
2024-01-29 15:43     ` Alexander Lobakin
2024-01-29 15:43       ` Alexander Lobakin
2024-01-31 17:52       ` Alan Brady
2024-01-31 17:52         ` Alan Brady

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=41774365-e247-4e32-9e96-d256acfcc129@intel.com \
    --to=aleksander.lobakin@intel.com \
    --cc=alan.brady@intel.com \
    --cc=igor.bagnucki@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=netdev@vger.kernel.org \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.