Message-ID: <f8bac3d8-4167-f9cc-5549-ff87a27f2155@kernel.dk>
Date:   Sun, 29 Jan 2023 14:53:41 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
Subject: Re: pktcdvd
Content-Language: en-US
To:     Linus Torvalds <torvalds@linux-foundation.org>,
        =?UTF-8?Q?Pali_Roh=c3=a1r?= <pali@kernel.org>
Cc:     Christoph Hellwig <hch@infradead.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
References: <CAHk-=wiDdw8tRzzx=ZBzUftC1TOiOO+kxv0s8HS342BC-jzkLQ@mail.gmail.com>
 <90eb90da-2679-cac0-979d-6ba0cc8ccbb8@kernel.dk>
 <20230105174210.jbjoqelllcrd57q6@pali>
 <58d3649f-3c8c-8b12-1930-f06f59837ad5@kernel.dk>
 <CAHk-=wiKUWm3VoYHK-oKixc9nF5Qdwp598MPSoh=jdxKAU1bOw@mail.gmail.com>
 <1933bddd-42d7-d92b-974f-f26c46c01547@kernel.dk>
 <CAHk-=wjJ=wD5D80hkWNCjJqS+djckAL+nXhXaHaiFzMAMve3rA@mail.gmail.com>
 <182bc0ee-51e3-b8c4-59f7-dcf702865a95@kernel.dk>
 <20230106165809.vkz7lr7gi3xce22e@pali>
 <fd143218-d8ba-e6c5-9225-b8e2aee09979@kernel.dk>
 <20230128193458.ukl35ev4mwbjmu6b@pali>
 <CAHk-=whfor49J0YTYi6zauiJ_MWwF-XwhSty+HvD4CzxFQ_ZGA@mail.gmail.com>
From:   Jens Axboe <axboe@kernel.dk>
In-Reply-To: <CAHk-=whfor49J0YTYi6zauiJ_MWwF-XwhSty+HvD4CzxFQ_ZGA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 1/28/23 12:43 PM, Linus Torvalds wrote:
> On Sat, Jan 28, 2023 at 11:35 AM Pali Rohár <pali@kernel.org> wrote:
>>
>> Hello! Sorry for a longer delay. Now I have started testing it with
>> Linux 6.2.0-rc5. Adding mapping works fine. Reading also works. Mounting
>> filesystem also works, reading mounted fs also. But after writing some
>> data to fs and calling sync cause kernel oops. Below is the dmesg log.
>> "sync" freezes and never finish.
>>
>> [ 1284.701497] pktcdvd: pktcdvd0: writer mapped to sr0
>> [ 1321.432589] pktcdvd: pktcdvd0: Fixed packets, 32 blocks, Mode-2 disc
>> [ 1321.437543] pktcdvd: pktcdvd0: maximum media speed: 10
>> [ 1321.437546] pktcdvd: pktcdvd0: write speed 10x
>> [ 1327.098955] pktcdvd: pktcdvd0: 590528kB available on disc
>> [ 1329.737263] UDF-fs: INFO Mounting volume 'LinuxUDF', timestamp 2023/01/28 19:16 (103c)
>> [ 1435.627449] ------------[ cut here ]------------
>> [ 1435.627466] kernel BUG at drivers/block/pktcdvd.c:2434!
> 
> Well, this is very much an example of one of the BUG_ON() cases I
> absolutely hate - not only did it cause the traceback (which can be
> interesting), it also effectively killed the machine in the process.
> 
> So that BUG_ON() most definitely shouldn't be a BUG_ON().
> 
> Turning it into a WARN_ON() (possibly even of the "ONCE" variety)
> together with then finishing the IO with a bio_io_error() would have
> been a better option for debugging.
> 
> Of course, the real fix is to fix whatever causes it, and I don't know
> what that is.
> 
> So I'm just piping up to once more highlight my hatred of using
> BUG_ON() for "this shouldn't happen" debug code. It's basically never
> the right thing to do unless you are in core code that would kill the
> machine anyway.

To be fair, this code is 20 years old... It should not be using
BUG_ON(), totally agree, but that it was quite common back in those
days.

-- 
Jens Axboe