From patchwork Mon Feb 25 10:20:21 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stanislaw Gruszka X-Patchwork-Id: 2180551 Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id B1FF53FCA4 for ; Mon, 25 Feb 2013 10:20:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759254Ab3BYKUT (ORCPT ); Mon, 25 Feb 2013 05:20:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:8941 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759233Ab3BYKUR (ORCPT ); Mon, 25 Feb 2013 05:20:17 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1PAK9iL025185 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 25 Feb 2013 05:20:16 -0500 Received: from localhost (vpn-231-206.phx2.redhat.com [10.3.231.206]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r1PAK8Cf025371; Mon, 25 Feb 2013 05:20:09 -0500 Date: Mon, 25 Feb 2013 11:20:21 +0100 From: Stanislaw Gruszka To: greearb@candelatech.com Cc: linux-wireless@vger.kernel.org Subject: Re: [PATCH] mac80211: Clean up work-queues on disassociation. Message-ID: <20130225102020.GA1735@redhat.com> References: <1361326267-16847-1-git-send-email-greearb@candelatech.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1361326267-16847-1-git-send-email-greearb@candelatech.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org On Tue, Feb 19, 2013 at 06:11:07PM -0800, greearb@candelatech.com wrote: > From: Ben Greear > > The monitor_work and beacon_connection_loss_work items were > not being canceled on disassociation (and not on deletion > either). This leads to work-items trying to run after memory > has been deleted. > > I could not find a cleaner way to do this because the > cancel_work_sync for these items must be done outside > of the ifmgd->mtx. > > In addition, re-order the quiesce code so that timers are > always stopped before work-items are flushed. This was > not the problem I saw, but I think it may still be more > correct. I think this patch is quite complicated and simpler solution can be used. We stop timers on disassociate, and since we nullify ifmgd->associated under ifmgd->mtx, work procedures will perform no action. The only thing we should care are works queued to workqueue internals, and stop them before we remove the interface, to stop workqueue code to use our freed ifmgd->*_work data. Regarding quiesce, I'm working on suspend/resume changes where this function become not necessary, so I'll remove it. Below is alternative fix proposition for the problem. diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index bdddb0b..15a24214 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -3992,6 +3992,17 @@ void ieee80211_mgd_stop(struct ieee80211_sub_if_data *sdata) { struct ieee80211_if_managed *ifmgd = &sdata->u.mgd; + /* + * We canceled timers during disassoc, but works still can be pending. + * Even if we they do not perform action when unassociated, we should + * assure we stop them, before freeing resources. + */ + cancel_work_sync(&ifmgd->request_smps_work); + cancel_work_sync(&ifmgd->monitor_work); + cancel_work_sync(&ifmgd->beacon_connection_loss_work); + cancel_work_sync(&ifmgd->csa_connection_drop_work); + cancel_work_sync(&ifmgd->chswitch_work); + mutex_lock(&ifmgd->mtx); if (ifmgd->assoc_data) ieee80211_destroy_assoc_data(sdata, false);