From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755064AbcFQAio (ORCPT ); Thu, 16 Jun 2016 20:38:44 -0400 Received: from mail-io0-f196.google.com ([209.85.223.196]:35303 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754991AbcFQAil (ORCPT ); Thu, 16 Jun 2016 20:38:41 -0400 Subject: Re: [BUG] act_ife: sleeping functions called in atomic context To: Cong Wang , Alexey Khoroshilov References: <1466110219-4825-1-git-send-email-khoroshilov@ispras.ru> Cc: "David S. Miller" , Linux Kernel Network Developers , LKML , ldv-project@linuxtesting.org From: Jamal Hadi Salim Message-ID: <57634687.3050107@mojatatu.com> Date: Thu, 16 Jun 2016 20:38:31 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------060300070900080504090708" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is a multi-part message in MIME format. --------------060300070900080504090708 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 16-06-16 05:43 PM, Cong Wang wrote: > On Thu, Jun 16, 2016 at 1:50 PM, Alexey Khoroshilov > wrote: >> tcf_ife_init() contains a big chunk of code executed with >> ife->tcf_lock spinlock held. But that code contains several calls >> to sleeping functions: >> populate_metalist() and use_all_metadata() >> -> add_metainfo() >> -> find_ife_oplist(metaid) >> -> read_lock() >> -> try_module_get(o->owner) >> -> kzalloc(sizeof(*mi), GFP_KERNEL); > > Hmm, we don't need to hold that spinlock when we create a new ife action, > since we haven't inserted it yet. We do need it when we modify an existing > one. So I am thinking if we can refactor that code to avoid spinlock > whenever possible. > Does attached (compile tested) patch help? >> -> ops->alloc(mi, metaval); >> -> module_put(ops->owner); >> _tcf_ife_cleanup() >> -> module_put() >> >> The same problem is actual for tcf_ife_cleanup() as well. >> > > Huh? Both module_put() and kfree() should not sleep, right? > I dont think there is any sleeping there ;-> cheers, jamal --------------060300070900080504090708 Content-Type: text/plain; charset=UTF-8; name="patchlet1" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="patchlet1" ZGlmZiAtLWdpdCBhL25ldC9zY2hlZC9hY3RfaWZlLmMgYi9uZXQvc2NoZWQvYWN0X2lmZS5j CmluZGV4IDZiYmM1MTguLmUzNDFiZWYgMTAwNjQ0Ci0tLSBhL25ldC9zY2hlZC9hY3RfaWZl LmMKKysrIGIvbmV0L3NjaGVkL2FjdF9pZmUuYwpAQCAtMzAyLDcgKzMwMiw5IEBAIHN0YXRp YyBpbnQgYWRkX21ldGFpbmZvKHN0cnVjdCB0Y2ZfaWZlX2luZm8gKmlmZSwgdTMyIG1ldGFp ZCwgdm9pZCAqbWV0YXZhbCwKIAkJfQogCX0KIAorCXNwaW5fbG9ja19iaCgmaWZlLT50Y2Zf bG9jayk7CiAJbGlzdF9hZGRfdGFpbCgmbWktPm1ldGFsaXN0LCAmaWZlLT5tZXRhbGlzdCk7 CisJc3Bpbl91bmxvY2tfYmgoJmlmZS0+dGNmX2xvY2spOwogCiAJcmV0dXJuIHJldDsKIH0K QEAgLTQ3NCw3ICs0NzYsNiBAQCBzdGF0aWMgaW50IHRjZl9pZmVfaW5pdChzdHJ1Y3QgbmV0 ICpuZXQsIHN0cnVjdCBubGF0dHIgKm5sYSwKIAkJCXNhZGRyID0gbmxhX2RhdGEodGJbVENB X0lGRV9TTUFDXSk7CiAJfQogCi0Jc3Bpbl9sb2NrX2JoKCZpZmUtPnRjZl9sb2NrKTsKIAlp ZmUtPnRjZl9hY3Rpb24gPSBwYXJtLT5hY3Rpb247CiAKIAlpZiAocGFybS0+ZmxhZ3MgJiBJ RkVfRU5DT0RFKSB7CkBAIC01MDQsNyArNTA1LDYgQEAgbWV0YWRhdGFfcGFyc2VfZXJyOgog CQkJaWYgKHJldCA9PSBBQ1RfUF9DUkVBVEVEKQogCQkJCV90Y2ZfaWZlX2NsZWFudXAoYSwg YmluZCk7CiAKLQkJCXNwaW5fdW5sb2NrX2JoKCZpZmUtPnRjZl9sb2NrKTsKIAkJCXJldHVy biBlcnI7CiAJCX0KIApAQCAtNTIzLDEzICs1MjMsMTAgQEAgbWV0YWRhdGFfcGFyc2VfZXJy OgogCQkJaWYgKHJldCA9PSBBQ1RfUF9DUkVBVEVEKQogCQkJCV90Y2ZfaWZlX2NsZWFudXAo YSwgYmluZCk7CiAKLQkJCXNwaW5fdW5sb2NrX2JoKCZpZmUtPnRjZl9sb2NrKTsKIAkJCXJl dHVybiBlcnI7CiAJCX0KIAl9CiAKLQlzcGluX3VubG9ja19iaCgmaWZlLT50Y2ZfbG9jayk7 Ci0KIAlpZiAocmV0ID09IEFDVF9QX0NSRUFURUQpCiAJCXRjZl9oYXNoX2luc2VydCh0biwg YSk7CiAK --------------060300070900080504090708--