Received-SPF: pass (google.com: domain of linux-kernel+bounces-85122-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <d8b52875-1eea-5eba-55d0-d6acedf206ef@quicinc.com>
Date: Wed, 28 Feb 2024 19:12:53 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH 1/4] devcoredump: Add dev_coredump_put()
Content-Language: en-US
To: =?UTF-8?Q?Jos=c3=a9_Roberto_de_Souza?= <jose.souza@intel.com>,
        <linux-kernel@vger.kernel.org>, <intel-xe@lists.freedesktop.org>
CC: Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Johannes Berg
	<johannes@sipsolutions.net>,
        Jonathan Cavitt <jonathan.cavitt@intel.com>
References: <20240227210008.182904-1-jose.souza@intel.com>
From: Mukesh Ojha <quic_mojha@quicinc.com>
In-Reply-To: <20240227210008.182904-1-jose.souza@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit


On 2/28/2024 2:30 AM, José Roberto de Souza wrote:
> This is useful for drivers that don't want to keep a coredump
> available after unloading.
> Otherwise, the coredump would only be removed after DEVCD_TIMEOUT
> seconds.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Mukesh Ojha <quic_mojha@quicinc.com>
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   drivers/base/devcoredump.c  | 22 ++++++++++++++++++++++
>   include/linux/devcoredump.h |  5 +++++
>   2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
> index 7e2d1f0d903a6..e96427411b87c 100644
> --- a/drivers/base/devcoredump.c
> +++ b/drivers/base/devcoredump.c
> @@ -304,6 +304,28 @@ static ssize_t devcd_read_from_sgtable(char *buffer, loff_t offset,
>   				  offset);
>   }
>  
[...]

> +/**
> + * dev_coredump_put - remove device coredump
> + * @dev: the struct device for the crashed device
> + *
> + * If giving device has a coredump this removes it from file system and free
> + * associated data otherwise does nothing.
> + * This is useful for drivers that don't want to keep a coredump
> + * available after unloading.
> + */

[...]

Slight rephrasing..

""
dev_coredump_put() removes coredump, if exists, for a given device from 
the file system and free its associated data otherwise, does nothing.

It is useful for modules that do not want to keep coredump
available after its unload.
""

Overall, LGTM..

-Mukesh

> +void dev_coredump_put(struct device *dev)
> +{
> +	struct device *existing;
> +
> +	existing = class_find_device(&devcd_class, NULL, dev,
> +				     devcd_match_failing);
> +	if (existing) {
> +		devcd_free(existing, NULL);
> +		put_device(existing);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(dev_coredump_put);
> +
>   /**
>    * dev_coredumpm - create device coredump with read/free methods
>    * @dev: the struct device for the crashed device
> diff --git a/include/linux/devcoredump.h b/include/linux/devcoredump.h
> index c008169ed2c6f..c8f7eb6cc1915 100644
> --- a/include/linux/devcoredump.h
> +++ b/include/linux/devcoredump.h
> @@ -63,6 +63,8 @@ void dev_coredumpm(struct device *dev, struct module *owner,
>   
>   void dev_coredumpsg(struct device *dev, struct scatterlist *table,
>   		    size_t datalen, gfp_t gfp);
> +
> +void dev_coredump_put(struct device *dev);
>   #else
>   static inline void dev_coredumpv(struct device *dev, void *data,
>   				 size_t datalen, gfp_t gfp)
> @@ -85,6 +87,9 @@ static inline void dev_coredumpsg(struct device *dev, struct scatterlist *table,
>   {
>   	_devcd_free_sgtable(table);
>   }
> +static inline void dev_coredump_put(struct device *dev)
> +{
> +}
>   #endif /* CONFIG_DEV_COREDUMP */
>   
>   #endif /* __DEVCOREDUMP_H */