On Thu, Jan 16, 2025 at 11:55:34AM +0300, Nazir Bilal Yavuz wrote:
> I checked clang 4 as well on the link you sent and it also fixes the
> warning there.
Confirmed here, so done this way.
--
Michael
signature.asc
Description: PGP signature
Hi,
On Thu, 16 Jan 2025 at 10:12, Bertrand Drouvot
wrote:
>
> Hi,
>
> On Thu, Jan 16, 2025 at 12:47:17AM -0500, Tom Lane wrote:
> > Michael Paquier writes:
> > > Not completely sure about the number of parenthesis, but I hope that
> > > this should be enough (extra set around io_op):
> > > +#def
On Thu, Jan 16, 2025 at 07:12:34AM +, Bertrand Drouvot wrote:
> From what I can see, the above proposal does (at least) silent the warning
> here (clang 5.0.1 and same as demoiselle): https://godbolt.org/z/cGosfzGne (we
> can see the warning by using the current define and that the warning is g
Hi,
On Thu, Jan 16, 2025 at 12:47:17AM -0500, Tom Lane wrote:
> Michael Paquier writes:
> > Not completely sure about the number of parenthesis, but I hope that
> > this should be enough (extra set around io_op):
> > +#define pgstat_is_ioop_tracked_in_bytes(io_op) \
> > + (((unsigned int) (io_o
Michael Paquier writes:
> Not completely sure about the number of parenthesis, but I hope that
> this should be enough (extra set around io_op):
> +#define pgstat_is_ioop_tracked_in_bytes(io_op) \
> + (((unsigned int) (io_op)) < IOOP_NUM_TYPES && \
> + ((unsigned int) (io_op)) >= IOOP_EXT
On Thu, Jan 16, 2025 at 12:18:38AM -0500, Tom Lane wrote:
> However, the macro does provide a convenient place to hang the
> warning comment about keeping it sync'd with the enum.
> Personally I'd keep the macro but move it to pgstat.h, close
> to the enum declaration, so that there's more than eps
Michael Paquier writes:
> Just for an assert, I would just remove the macro rather than have an
> inline function.
Oh, I'd not noticed that there is only one caller.
However, the macro does provide a convenient place to hang the
warning comment about keeping it sync'd with the enum.
Personally I
On Wed, Jan 15, 2025 at 11:34:14PM -0500, Tom Lane wrote:
> Michael Paquier writes:
> > I cannot reproduce that, perhaps I'm just missing something with these
> > switches. Do you think that a cast would cool things? Please see the
> > attached for the idea.
>
> There are only three animals sho
Michael Paquier writes:
> I cannot reproduce that, perhaps I'm just missing something with these
> switches. Do you think that a cast would cool things? Please see the
> attached for the idea.
There are only three animals showing this warning (ayu, batfish,
demoiselle) so it likely requires par
On Wed, Jan 15, 2025 at 12:19:03PM -0500, Tom Lane wrote:
> I don't see a reasonable way to alter that check to suppress this;
> for instance, "(io_op) <= IOOP_WRITE" would probably still draw the
> same warning. I think most likely we have to remove that check, ie
>
> #define pgstat_is_ioop_tra
Nazir Bilal Yavuz writes:
> On Tue, 14 Jan 2025 at 06:18, Michael Paquier wrote:
>> And I've somewhat managed to fat-finger the business with
>> pgstat_count_io_op() with an incorrect rebase. Will remove in a
>> minute..
> Thank you!
Commit f92c854cf has caused some of the buildfarm members to
Hi,
On Tue, 14 Jan 2025 at 06:18, Michael Paquier wrote:
>
> On Fri, Jan 10, 2025 at 08:23:46AM +, Bertrand Drouvot wrote:
> > On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote:
> >> But I agree that having a macro has more benefits,
> >> also there already is a check for the
On Fri, Jan 10, 2025 at 08:23:46AM +, Bertrand Drouvot wrote:
> On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote:
>> But I agree that having a macro has more benefits,
>> also there already is a check for the 'io_op < IOOP_NUM_TYPES' in the
>> pgstat_count_io_op() function.
>
Hi,
On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote:
> Hi,
>
> On Fri, 10 Jan 2025 at 04:47, Michael Paquier wrote:
> >
> > On Thu, Jan 09, 2025 at 03:30:37PM +, Bertrand Drouvot wrote:
> > > We first use an Assert in is_ioop_tracked_in_bytes() and then we return
> > > a va
Hi,
On Fri, 10 Jan 2025 at 04:47, Michael Paquier wrote:
>
> On Thu, Jan 09, 2025 at 03:30:37PM +, Bertrand Drouvot wrote:
> > We first use an Assert in is_ioop_tracked_in_bytes() and then we return
> > a value "just" to check another Assert. I wonder if it wouldn't make more
> > sense
> > t
On Thu, Jan 09, 2025 at 03:30:37PM +, Bertrand Drouvot wrote:
> We first use an Assert in is_ioop_tracked_in_bytes() and then we return
> a value "just" to check another Assert. I wonder if it wouldn't make more
> sense
> to get rid of this function and use a macro instead, something like?
>
Hi,
On Thu, Jan 09, 2025 at 02:20:16PM +0300, Nazir Bilal Yavuz wrote:
> Hi,
>
> On Thu, 9 Jan 2025 at 11:11, Michael Paquier wrote:
> >
> > On Thu, Jan 09, 2025 at 10:15:20AM +0300, Nazir Bilal Yavuz wrote:
> > > I am a bit confused, are you suggesting these two alternatives:
> > > 1- Making pg
Hi,
On Thu, 9 Jan 2025 at 11:11, Michael Paquier wrote:
>
> On Thu, Jan 09, 2025 at 10:15:20AM +0300, Nazir Bilal Yavuz wrote:
> > I am a bit confused, are you suggesting these two alternatives:
> > 1- Making pgstat_count_io_op_n() static and continuing to use
> > pgstat_count_io_op() as it is.
>
On Thu, Jan 09, 2025 at 10:15:20AM +0300, Nazir Bilal Yavuz wrote:
> I am a bit confused, are you suggesting these two alternatives:
> 1- Making pgstat_count_io_op_n() static and continuing to use
> pgstat_count_io_op() as it is.
> 2- Removing pgstat_count_io_op() and instead using
> pgstat_count_i
Hi,
On Thu, 9 Jan 2025 at 10:15, Nazir Bilal Yavuz wrote:
>
> On Thu, 9 Jan 2025 at 05:59, Michael Paquier wrote:
> >
> >
> > +static inline bool
> > +is_ioop_tracked_in_bytes(IOOp io_op)
> > +{
> > +Assert((unsigned int) io_op < IOOP_NUM_TYPES);
> > +return io_op >= IOOP_EXTEND;
> > +}
Hi,
Thanks for the review!
On Thu, 9 Jan 2025 at 05:59, Michael Paquier wrote:
>
> On Thu, Dec 26, 2024 at 02:41:26PM +0300, Nazir Bilal Yavuz wrote:
> > Thanks! v4 is attached. I quickly tested the pg_stat_get_backend_io()
> > function and it seems it is working.
>
> Thanks a lot for the rebase
On Thu, Dec 26, 2024 at 02:41:26PM +0300, Nazir Bilal Yavuz wrote:
> Thanks! v4 is attached. I quickly tested the pg_stat_get_backend_io()
> function and it seems it is working.
Thanks a lot for the rebased version. This looks pretty solid. Here
are some comments.
void
-pgstat_count_io_op(IOOb
Hi,
On Tue, 24 Dec 2024 at 09:13, Michael Paquier wrote:
>
> On Fri, Dec 06, 2024 at 12:41:55PM +0300, Nazir Bilal Yavuz wrote:
> > Thanks! I think 'track' is a better word in this context. I used
> > 'tracked in ...', as it sounded more correct to me (I hope it is).
>
> Splitting op_bytes into t
On Fri, Dec 06, 2024 at 12:41:55PM +0300, Nazir Bilal Yavuz wrote:
> Thanks! I think 'track' is a better word in this context. I used
> 'tracked in ...', as it sounded more correct to me (I hope it is).
Splitting op_bytes into three fields sounds like a good idea. Count
me in regarding the concep
Hi,
On Thu, 5 Dec 2024 at 12:13, Bertrand Drouvot
wrote:
>
> Hi,
>
> On Wed, Dec 04, 2024 at 02:49:11PM +0300, Nazir Bilal Yavuz wrote:
> > On Thu, 28 Nov 2024 at 16:39, Bertrand Drouvot
> > wrote:
> > >
> > You are right, no need to have this check; it can not be less than 0.
> > I completely r
Hi,
On Wed, Dec 04, 2024 at 02:49:11PM +0300, Nazir Bilal Yavuz wrote:
> On Thu, 28 Nov 2024 at 16:39, Bertrand Drouvot
> wrote:
> >
> You are right, no need to have this check; it can not be less than 0.
> I completely removed the function now.
Thanks! Yeah I think that makes sense.
> > What
Hi,
Thanks for looking into this!
On Thu, 28 Nov 2024 at 16:39, Bertrand Drouvot
wrote:
>
> Hi,
>
> On Wed, Nov 27, 2024 at 11:08:01AM -0500, Melanie Plageman wrote:
> > On Wed, Sep 11, 2024 at 7:19 AM Nazir Bilal Yavuz
> > wrote:
> > >
> > > Currently, in the pg_stat_io view, IOs are counted
Hi,
On Wed, Nov 27, 2024 at 11:08:01AM -0500, Melanie Plageman wrote:
> On Wed, Sep 11, 2024 at 7:19 AM Nazir Bilal Yavuz wrote:
> >
> > Currently, in the pg_stat_io view, IOs are counted as blocks. However,
> > there are two issues with this approach:
> >
> > 1- The actual number of IO requests
On Wed, Sep 11, 2024 at 7:19 AM Nazir Bilal Yavuz wrote:
>
> Currently, in the pg_stat_io view, IOs are counted as blocks. However, there
> are two issues with this approach:
>
> 1- The actual number of IO requests to the kernel is lower because IO
> requests can be merged before sending the fin
Hi,
Currently, in the pg_stat_io view, IOs are counted as blocks. However,
there are two issues with this approach:
1- The actual number of IO requests to the kernel is lower because IO
requests can be merged before sending the final request. Additionally, it
appears that all IOs are counted in b
30 matches
Mail list logo
