All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Andrey Tsyvarev <tsyvarev@ispras.ru>
Cc: linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH] kernel/module.c: Free lock-classes if parse_args failed
Date: Thu, 22 Jan 2015 11:10:30 +1030	[thread overview]
Message-ID: <87bnlrwsu9.fsf@rustcorp.com.au> (raw)
In-Reply-To: <54BF8439.2000905@ispras.ru>

Andrey Tsyvarev <tsyvarev@ispras.ru> writes:
> 21.01.2015 4:40, Rusty Russell пишет:
>> Andrey Tsyvarev <tsyvarev@ispras.ru> writes:
>>> 20.01.2015 9:37, Rusty Russell пишет:
>>>> Andrey Tsyvarev <tsyvarev@ispras.ru> writes:
>>>>> parse_args call module parameters' .set handlers, which may use locks defined in the module.
>>>>> So, these classes should be freed in case parse_args returns error(e.g. due to incorrect parameter passed).
>>>> Thanks, this seems right.  Applied.
>>>>
>>>> But this makes me ask: where is lockdep_free_key_range() called on the
>>>> module init code?  It doesn't seem to be at all...
>>> As I understand, locks are not allowed to be defined in the module init
>>> section. So, no needs to call lockdep_free_key_range() for it.
>>> This has a sense: objects from that section are allowed to be used only
>>> by module->init() function. But a single function call doesn't require
>>> any synchronization wrt itself.
>> I don't know that we have any __initdata locks; it would be really
>> weird.
>>
>> But change 'static DEFINE_MUTEX(mutex_param);' to 'static __initdata
>> DEFINE_MUTEX(mutex_param);' to test.
> Compiler warns about sections mismatch, but the test works.
>
> According to lockdep_free_key_range() code, lock class is cleared not 
> only according to
> its key(which is equal to lock address in the case of static lock) but 
> also according to its name.

What happens if you later register another lock at that address, since
the memory is freed?

A quick grep revealed no __initdata locks in the kernel, so I don't
think we care anyway.

Cheers,
Rusty.

  reply	other threads:[~2015-01-22  1:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-14  6:25 [PATCH] kernel/module.c: Free lock-classes if parse_args failed Andrey Tsyvarev
2015-01-20  6:37 ` Rusty Russell
2015-01-20  7:47   ` Andrey Tsyvarev
2015-01-21  1:40     ` Rusty Russell
2015-01-21 10:49       ` Andrey Tsyvarev
2015-01-22  0:40         ` Rusty Russell [this message]
2015-01-22  9:27           ` Andrey Tsyvarev
2015-01-20  9:48   ` Peter Zijlstra
2015-02-19  0:12     ` Ingo Molnar
2015-02-19 11:57       ` Peter Zijlstra
2015-02-19 12:24         ` Ingo Molnar

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=87bnlrwsu9.fsf@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tsyvarev@ispras.ru \
    /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.