All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: <Arun.Ramadoss@microchip.com>
Cc: <andrew@lunn.ch>, <olteanv@gmail.com>,
	<linux-kernel@vger.kernel.org>, <george.mccollister@gmail.com>,
	<vivien.didelot@gmail.com>, <UNGLinuxDriver@microchip.com>,
	<f.fainelli@gmail.com>, <netdev@vger.kernel.org>,
	<Woojung.Huh@microchip.com>, <davem@davemloft.net>
Subject: Re: [PATCH net] net: dsa: microchip: Added the condition for scheduling ksz_mib_read_work
Date: Mon, 11 Oct 2021 06:45:38 -0700	[thread overview]
Message-ID: <20211011064538.7fabf949@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <601a427d9d73ef7aa85e50770cce38ecd6e84463.camel@microchip.com>

On Mon, 11 Oct 2021 09:41:55 +0000 Arun.Ramadoss@microchip.com wrote:
> On Fri, 2021-10-08 at 11:34 -0700, Jakub Kicinski wrote:
> > Also the cancel_delayed_work_sync() is suspiciously early in the
> > remove
> > flow. There is a schedule_work call in ksz_mac_link_down() which may
> > schedule the work back in. That'd also explain why the patch helps
> > since
> > ksz_mac_link_down() only schedules if (dev->mib_read_interval).  
> In this patch, I did two things. Added the if condition for
> rescheduling the queue and other is resetted the mib_read_interval to
> zero.
> As per the cancel_delayed_queue_sync() functionality, Now I tried rmod
> after removing the if condition for resheduling the queue,kernel didn't
> crash. So, concluded that it is not rearm in ksz_mib_read_work  is
> causing the problem but it is due to scheduling in the
> ksz_mac_link_down function. This function is called due to the
> dsa_unregister_switch. Due to resetting of the mib_read_interval to
> zero in switch_remove, the queue is not scheduled in mac_link_down, so
> kernel didn't crash.
> 
> And also, as per suggestion on cancel_delayed_work_sync() is
> suspiciously placed in switch_remove. I undo this patch, and tried just
> by moving the canceling of delayed_work after the dsa_unregister_switch
> function. As expected dsa_unregister_switch calls the
> ksz_mac_link_down, which schedules the mib_read_work. Now, when
> cancel_delayed_work_sync is called, it cancels all the workqueue. As a
> result, module is removed successfully without kernel crash.
> 
> Can I send the updated patch as v1 or new patch with updated commit
> message and description.

Please send a patch with just the second chunk (zeroing
mib_read_interval), you can mark it as v2.

      reply	other threads:[~2021-10-11 13:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-08  8:43 [PATCH net] net: dsa: microchip: Added the condition for scheduling ksz_mib_read_work Arun Ramadoss
2021-10-08 13:58 ` Andrew Lunn
2021-10-08 18:34   ` Jakub Kicinski
2021-10-11  9:41     ` Arun.Ramadoss
2021-10-11 13:45       ` Jakub Kicinski [this message]

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=20211011064538.7fabf949@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=Arun.Ramadoss@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=Woojung.Huh@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=george.mccollister@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=vivien.didelot@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.