Re: [Intel-wired-lan] [PATCH v3 net] ice: Write all GNSS buffers instead of first one

From: Michal Schmidt <mschmidt@redhat.com>
To: Karol Kolacinski <karol.kolacinski@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>,
	intel-wired-lan@lists.osuosl.org
Subject: Re: [Intel-wired-lan] [PATCH v3 net] ice: Write all GNSS buffers instead of first one
Date: Fri, 24 Mar 2023 17:47:59 +0100	[thread overview]
Message-ID: <CADEbmW0yGTU3u-zznKprkS8Jc=JzvZWMC36wYfszc4SAZBYE5A@mail.gmail.com> (raw)
In-Reply-To: <20230217120541.16745-1-karol.kolacinski@intel.com>

On Fri, Feb 17, 2023 at 1:06 PM Karol Kolacinski
<karol.kolacinski@intel.com> wrote:
>
> When user writes multiple messages in a short period of time, the driver
> writes only the first one and buffers others in a linked list.
>
> Fix this behavior to write all pending buffers instead of only the first
> one.
>
> To reproduce this issue, open the GNSS device with cat, send a few
> messages to the device, e.g. multiple commands using echo. The issue
> manifests itself as response to only first message. Then, after issuing
> a single or multiple commands, user can see that response from the
> device was not for recent ones but for the next single buffered one from
> the first batch.

Although the patch does fix the described issue in my testing,
I believe the buffering must be eliminated.
See my patch "ice: make writes to /dev/gnssX synchronous",
https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20230324162056.200752-1-mschmidt@redhat.com/

Michal

> Fixes: d6b98c8d242a ("ice: add write functionality for GNSS TTY")
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> ---
> V2 -> V3: Switched to net-queue tree instead of next-queue.
> V1 -> V2: Added reproduction steps in the commit message.
>
>  drivers/net/ethernet/intel/ice/ice_gnss.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_gnss.c b/drivers/net/ethernet/intel/ice/ice_gnss.c
> index 43e199b5b513..02533014f24a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_gnss.c
> +++ b/drivers/net/ethernet/intel/ice/ice_gnss.c
> @@ -82,7 +82,7 @@ static void ice_gnss_write_pending(struct kthread_work *work)
>                                                 write_work);
>         struct ice_pf *pf = gnss->back;
>
> -       if (!list_empty(&gnss->queue)) {
> +       while (!list_empty(&gnss->queue)) {
>                 struct gnss_write_buf *write_buf = NULL;
>                 unsigned int bytes;
>
> --
> 2.37.2
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan